[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-15 Thread xccui 
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5210
  
Hi @sunjincheng121, thanks for your reply. I think an example would be 
that, for some non-standard CSV files like `a, b , c,`, if the boolean flag 
`trailingDelimiter=false`, the file will be parsed with 4 fields; while if 
`trailingDelimiter=true`, the file will be parsed with 3 fields, in which the 
trailing delimiter `,` is omitted. Further, the trailing delimiter could be set 
as another character, e.g., `a, b, c;`.


---

[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-15 Thread sunjincheng121 
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/5210
  
@xccui sorry for the late reply.  Sometimes we add new feature when we have 
the requirement. :) Can you give me a example about  add the trailing delimiter 
as a new feature? 

Thanks, Jincheng


---

[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-09 Thread xccui 
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5210
  
Hi @sunjincheng121, yes, the reported issue should be solved with 
FLINK-8331. In addition, maybe we can add the trailing delimiter as a new 
feature (like the 
[CSVParser.java](https://github.com/apache/commons-csv/blob/master/src/main/java/org/apache/commons/csv/CSVParser.java)
 and 
[CSVFormat.java](https://github.com/apache/commons-csv/blob/master/src/main/java/org/apache/commons/csv/CSVFormat.java#L559)
 in commons-csv). I'll do some refactorings to allow separate trailing 
delimiters for both the input and output. What do you think?


---

[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-09 Thread sunjincheng121 
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/5210
  
Hi, @xccui thanks for tell me more about the background of this issue. 
But if we improved the `CsvInputFormat` to support that case. Just like 
FLINK-8331 did.  I think we do not need this change, What do you think?

Best, Jincheng


---

[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-08 Thread xccui 
Github user xccui commented on the issue:

https://github.com/apache/flink/pull/5210
  
Hi @sunjincheng121, thanks for the review! You may refer to [this 
thread](https://lists.apache.org/thread.html/cfe3b1718a479300dc91d1523be023ef5bc702bd5ad53af4fea5a596@%3Cuser.flink.apache.org%3E)
 for further background of the issue.

Thanks, Xingcan


---

[GitHub] flink issue #5210: [FLINK-8316] [table] The CsvTableSink and the CsvInputFor...

2018-01-08 Thread sunjincheng121 
Github user sunjincheng121 commented on the issue:

https://github.com/apache/flink/pull/5210
  
Thanks for the PR @xccui. The PR looks good for me. But I am not sure, do 
we need this change if all CSV parser supported the format of without trailing 
delimiter.  What do you think? 


---