xiedeyantu commented on code in PR #4893:
URL: https://github.com/apache/calcite/pull/4893#discussion_r3182781299
##########
example/csv/src/main/java/org/apache/calcite/adapter/csv/CsvScannableTable.java:
##########
@@ -58,7 +58,7 @@ public class CsvScannableTable extends CsvTable
return new AbstractEnumerable<@Nullable Object[]>() {
@Override public Enumerator<@Nullable Object[]> enumerator() {
return new CsvEnumerator<>(source, cancelFlag, false, null,
- CsvEnumerator.arrayConverter(fieldTypes, fields, false));
+ CsvEnumerator.arrayConverter(fieldTypes, fields, false), ',');
Review Comment:
If you prefer not to add the constructor, I'm fine with that as long as the
functionality is met. I was just trying to say that adding a constructor could
save us from a lot of changes (although in this PR, even though a new
constructor was added, some unnecessary changes weren't removed). I think this
PR is ready if we revert it to the previous version(no new constructor). What
do you think? @julianhyde
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
