You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/05/22 09:34:40 UTC
[GitHub] [incubator-kyuubi] link3280 opened a new pull request, #2718: [KYUUBI#2405] Support Flink StringData Data Type
link3280 opened a new pull request, #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718
### _Why are the changes needed?_
Currently, Flink uses its legacy data type system in CollectSink, but sooner would move to the new type system. Kyuubi should adapt to the new data type system beforehand.
This PR supports StringData in Flink.
This is a subtask of #2100 .
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878832262
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -136,18 +137,22 @@ object RowSet {
tDoubleValue.setValue(row.getField(ordinal).asInstanceOf[Double])
}
TColumnValue.doubleVal(tDoubleValue)
- case _: VarCharType =>
+ case t @ (_: VarCharType | _: CharType) =>
val tStringValue = new TStringValue
- if (row.getField(ordinal) != null) {
- val stringValue = row.getField(ordinal).asInstanceOf[String]
- tStringValue.setValue(stringValue)
- }
- TColumnValue.stringVal(tStringValue)
- case _: CharType =>
- val tStringValue = new TStringValue
- if (row.getField(ordinal) != null) {
- val stringValue = row.getField(ordinal).asInstanceOf[String]
- tStringValue.setValue(stringValue)
+ val fieldValue = row.getField(ordinal)
+ fieldValue match {
+ case _: String =>
+ val stringValue = fieldValue.asInstanceOf[String]
+ tStringValue.setValue(stringValue)
Review Comment:
```suggestion
case value: String =>
tStringValue.setValue(value)
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 closed pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
URL: https://github.com/apache/incubator-kyuubi/pull/2718
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#issuecomment-1134353734
Thanks, merging to master
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878891446
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
Sorry for the misleading outdated comment. Updating the comment and the codes.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878875965
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
@yanghua I'd like to. Do you have a good idea? I think the key is that we can't get the underlying conversion class unless we read one value.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878883819
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
> But if we get a null value, we still can't determine its conversion class. We need a non-null value here, that's what the loop is for.
Oh... I misunderstood, based on your comment. Now, I suggest, you change your comment:
`by sampling the first row` -> `by sampling the first non-null row`.
So the suggestion may be to simplify the code style about how to find the first non-null value right?
May you reference this [answer](https://stackoverflow.com/a/8715730) in the StackOverflow?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 closed pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 closed pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
URL: https://github.com/apache/incubator-kyuubi/pull/2718
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 commented on PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#issuecomment-1133860327
@pan3793 Sure. Updated.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#issuecomment-1133890892
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2718](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (773d860) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/45531f01f5f74283aac266bda026414db8d01c28?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45531f0) will **increase** coverage by `0.08%`.
> The diff coverage is `70.96%`.
```diff
@@ Coverage Diff @@
## master #2718 +/- ##
============================================
+ Coverage 64.82% 64.91% +0.08%
Complexity 279 279
============================================
Files 409 409
Lines 19661 19677 +16
Branches 2638 2641 +3
============================================
+ Hits 12745 12773 +28
+ Misses 5706 5682 -24
- Partials 1210 1222 +12
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/kyuubi/engine/flink/schema/RowSet.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9zY2hlbWEvUm93U2V0LnNjYWxh) | `66.52% <70.96%> (+11.66%)` | :arrow_up: |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `52.63% <0.00%> (-7.02%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `68.96% <0.00%> (-2.30%)` | :arrow_down: |
| [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `68.00% <0.00%> (-1.34%)` | :arrow_down: |
| [.../engine/spark/session/SparkSQLSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zZXNzaW9uL1NwYXJrU1FMU2Vzc2lvbk1hbmFnZXIuc2NhbGE=) | `79.01% <0.00%> (-1.24%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `84.27% <0.00%> (-0.63%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [45531f0...773d860](https://codecov.io/gh/apache/incubator-kyuubi/pull/2718?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878879713
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
> @yanghua I'd like to. Do you have a good idea? I think the key is that we can't get the underlying conversion class unless we read one value.
I mean simplifying the way that we set the value for `sampleField ` variable.
The pseudo-code looks like this:
```
if (rows.length > 0) {
sampleField = rows(0)..getField(ordinal)
}
```
Can we remove the `while` block and `i` counter?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878879713
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
> @yanghua I'd like to. Do you have a good idea? I think the key is that we can't get the underlying conversion class unless we read one value.
I mean simplifying the way that we set the value for `sampleField ` variable.
The pseudo-code looks like this:
```
if (rows.length > 0) {
sampleField = rows(0).getField(ordinal)
}
```
Can we remove the `while` block and `i` counter?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878881946
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
But if we get a null value, we still can't determine its conversion class. We need a non-null value here, that's what the loop is for.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#issuecomment-1133856794
Thanks @link3280, could you please add the FLINK JIRA ticket link about the change you described?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] link3280 commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
link3280 commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878875965
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
@yanghua I'd like to. Do you have a good idea? The problem is that we can't get the underlying conversion class unless we read one value.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yanghua commented on a diff in pull request #2718: [KYUUBI#2405] Support Flink StringData Data Type
Posted by GitBox <gi...@apache.org>.
yanghua commented on code in PR #2718:
URL: https://github.com/apache/incubator-kyuubi/pull/2718#discussion_r878861731
##########
externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/schema/RowSet.scala:
##########
@@ -165,6 +169,14 @@ object RowSet {
private def toTColumn(rows: Seq[Row], ordinal: Int, logicalType: LogicalType): TColumn = {
val nulls = new java.util.BitSet()
+ // for each column, determine the conversion class by sampling the first row
+ // if there's zero row, set empty as the column
+ var sampleField: Object = null
+ var i = -1
+ while (sampleField == null && i + 1 < rows.length) {
+ i += 1
+ sampleField = rows(i).getField(ordinal)
+ }
Review Comment:
Can we simply this code snippet via an `if/else` block?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org