You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/10/18 16:25:21 UTC
[GitHub] [iceberg] stevenzwu opened a new pull request #1625: move convertConstant method from the base DataIterator to RowDataIter…
stevenzwu opened a new pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625
…ator, since it seems to be tied to Flink table row data types (like DecimalData, StringData, TimestampData etc.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-712283670
@JingsongLi I have moved the convertConstant method to `RowDataUtil` as you suggested. Also updated and rebased the PR.
@openinx yes, internally we have a similar iterator/iterable for Avro GenericRecord type. It is not much code since Iceberg library provided those reader functions already, like `ParquetAvroValueReaders`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] JingsongLi merged pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
JingsongLi merged pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] JingsongLi commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711494281
I see, My previous idea is that it is within the scope of `RowData`. For example, for Spark, `ColumnarBatch` also follows the interface of `InternalRow`. But I am OK to make it more generic.
Can you move this `convertConstant` to a new util class, like `org.apache.iceberg.flink.data.RowDataUtils`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711412980
@JingsongLi @openinx can you help take a look at this small refactoring?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu edited a comment on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-712283670
@JingsongLi I have moved the convertConstant method to `RowDataUtil` as you suggested. Also updated and rebased the PR.
@openinx yes, internally we have a similar iterator/iterable for Avro GenericRecord type. It is not much code since Iceberg library provided those reader functions already, like `ParquetAvroValueReaders`. For the future FLIP-27 source, we can reuse the `DataIterator` base class/interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu edited a comment on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711490862
@JingsongLi `DataIterator<T>` is for generic type T, which could be an Avro GenericRecord or sth else. Currently, `convertConstant` method deals with Flink data types used by `RowData` (like DecimalData, StringData, TimestampData etc.). Hence, I was thinking `RowDataIterator` is the right home for it. If there is a new `BatchDataIterator` impl for a different type `T`, I would imagine that we may need a diff impl of `convertConstant `.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] JingsongLi commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
JingsongLi commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711472831
Why move to `RowDataIter`? If we have `BatchDataIterator`, it may need to rely on this `convertConstant`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711490862
@JingsongLi `DataIterator<T>` is for generic type T, which could be an Avro GenericRecord or sth else. Currently, `convertConstant` method deals with Flink data types used by `RowData` (like DecimalData, StringData, TimestampData etc.). If there is a new `BatchDataIterator` impl for a different type `T`, I would imagine that we may need a diff impl of `convertConstant `.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #1625: Flink source: move convertConstant method from the base DataIterator to RowDataIter…
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #1625:
URL: https://github.com/apache/iceberg/pull/1625#issuecomment-711652322
> @JingsongLi DataIterator<T> is for generic type T, which could be an Avro GenericRecord or sth else.
I guess you are implementing the native avro reader for flink ? It's right that moving this RowData specific convertion out of `DataIteraotr<T>`. For me, moving to `RowDataUtils` or `RowDataIterator` , both work for me.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org