You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "turboFei (via GitHub)" <gi...@apache.org> on 2023/02/20 05:52:22 UTC
[GitHub] [kyuubi] turboFei opened a new pull request, #4372: Support to return null value for OperationsResource rowset
turboFei opened a new pull request, #4372:
URL: https://github.com/apache/kyuubi/pull/4372
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _How was this patch tested?_
- [ ] 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.readthedocs.io/en/master/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] [kyuubi] bowenliang123 closed pull request #4372: Support to return null value for OperationsResource rowset
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 closed pull request #4372: Support to return null value for OperationsResource rowset
URL: https://github.com/apache/kyuubi/pull/4372
--
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] [kyuubi] turboFei commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111552357
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
so, to keep the style align.
I decided to use `i.get*Val.isSetValue` finally.
--
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] [kyuubi] bowenliang123 commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111599536
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
Yeah, I have tried but failed before raising the original comment. Let's keep it as it is.
--
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] [kyuubi] turboFei commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436372988
cc @pan3793
--
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] [kyuubi] codecov-commenter commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436497449
# [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4372?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 [#4372](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b15f1eb) into [master](https://codecov.io/gh/apache/kyuubi/commit/5b2c40659f25d6222c96bff9e144f7bd5719b18c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5b2c406) will **increase** coverage by `0.00%`.
> The diff coverage is `71.42%`.
```diff
@@ Coverage Diff @@
## master #4372 +/- ##
=========================================
Coverage 53.70% 53.70%
Complexity 13 13
=========================================
Files 562 563 +1
Lines 30834 30877 +43
Branches 4157 4164 +7
=========================================
+ Hits 16558 16584 +26
- Misses 12728 12740 +12
- Partials 1548 1553 +5
```
| [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ache/kyuubi/server/api/v1/OperationsResource.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL09wZXJhdGlvbnNSZXNvdXJjZS5zY2FsYQ==) | `72.44% <71.42%> (+4.59%)` | :arrow_up: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `59.09% <0.00%> (-7.58%)` | :arrow_down: |
| [...in/java/org/apache/kyuubi/client/BatchRestApi.java](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXJlc3QtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9reXV1YmkvY2xpZW50L0JhdGNoUmVzdEFwaS5qYXZh) | `97.56% <0.00%> (-2.44%)` | :arrow_down: |
| [...mon/src/main/scala/org/apache/kyuubi/Logging.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9Mb2dnaW5nLnNjYWxh) | `41.25% <0.00%> (-1.25%)` | :arrow_down: |
| [...che/kyuubi/server/KyuubiTHttpFrontendService.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpVEh0dHBGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `60.00% <0.00%> (-0.69%)` | :arrow_down: |
| [...g/apache/kyuubi/operation/BatchJobSubmission.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQmF0Y2hKb2JTdWJtaXNzaW9uLnNjYWxh) | `75.27% <0.00%> (-0.55%)` | :arrow_down: |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `97.45% <0.00%> (-0.07%)` | :arrow_down: |
| [.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9LeXV1YmlDb25uZWN0aW9uLmphdmE=) | `5.75% <0.00%> (-0.01%)` | :arrow_down: |
| [.../org/apache/kyuubi/config/KyuubiReservedKeys.scala](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpUmVzZXJ2ZWRLZXlzLnNjYWxh) | `0.00% <0.00%> (ø)` | |
| [...va/org/apache/kyuubi/client/util/VersionUtils.java](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXJlc3QtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9reXV1YmkvY2xpZW50L3V0aWwvVmVyc2lvblV0aWxzLmphdmE=) | `63.63% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/kyuubi/pull/4372?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] [kyuubi] bowenliang123 commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436418968
> LGTM. Good catch.
>
> And can we tract a general method `getFieldValueOrNull`for this ?
--
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] [kyuubi] turboFei commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111546843
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
I tried to use that, but it does not work. you can have a try.
--
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] [kyuubi] bowenliang123 commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111542061
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
A possible risk is that `getStringVal` on the unset field will throw RuntimeException here.
Is that better with `i.isSetStringVal` for the case condition ? And make null as default return.
--
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] [kyuubi] turboFei commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111551805
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
With
```
case TColumnValue._Fields.STRING_VAL =>
if (i.isSetStringVal) {
i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.BOOL_VAL =>
if (i.isSetBoolVal) {
i.getBoolVal.getFieldValue(TBoolValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.BYTE_VAL =>
if (i.isSetByteVal) {
i.getByteVal.getFieldValue(TByteValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.DOUBLE_VAL =>
if (i.isSetDoubleVal) {
i.getDoubleVal.getFieldValue(TDoubleValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I16_VAL =>
if (i.isSetI16Val) {
i.getI16Val.getFieldValue(TI16Value._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I32_VAL =>
if (i.isSetI32Val) {
i.getI32Val.getFieldValue(TI32Value._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I64_VAL =>
if (i.isSetI64Val) {
i.getI64Val.getFieldValue(TI64Value._Fields.VALUE)
} else {
null
}
```
The result is:
```
ArrayBuffer(null, false, 0, 0.0, 0, 0, 0)
```
--
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] [kyuubi] turboFei closed pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #4372: Support to return null value for OperationsResource rowset
URL: https://github.com/apache/kyuubi/pull/4372
--
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] [kyuubi] turboFei commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111551805
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
With
```
case TColumnValue._Fields.STRING_VAL =>
if (i.isSetStringVal) {
i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.BOOL_VAL =>
if (i.isSetBoolVal) {
i.getBoolVal.getFieldValue(TBoolValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.BYTE_VAL =>
if (i.isSetByteVal) {
i.getByteVal.getFieldValue(TByteValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.DOUBLE_VAL =>
if (i.isSetDoubleVal) {
i.getDoubleVal.getFieldValue(TDoubleValue._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I16_VAL =>
if (i.isSetI16Val) {
i.getI16Val.getFieldValue(TI16Value._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I32_VAL =>
if (i.isSetI32Val) {
i.getI32Val.getFieldValue(TI32Value._Fields.VALUE)
} else {
null
}
case TColumnValue._Fields.I64_VAL =>
if (i.isSetI64Val) {
i.getI64Val.getFieldValue(TI64Value._Fields.VALUE)
} else {
null
}
```
The result is:
```
ArrayBuffer(null, false, 0, 0.0, 0, 0, 0)
```
--
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] [kyuubi] turboFei commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436421452
> And can we tract a general method getFieldValueOrNullfor this ?
@bowenliang123 I thought about that, but it only be used for one time.
--
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] [kyuubi] turboFei commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436532790
thanks, merging to master and branch-1.7
--
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] [kyuubi] turboFei commented on pull request #4372: Support to return null value for OperationsResource rowset
Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#issuecomment-1436410524
cc @simon824
--
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] [kyuubi] bowenliang123 commented on a diff in pull request #4372: Support to return null value for OperationsResource rowset
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4372:
URL: https://github.com/apache/kyuubi/pull/4372#discussion_r1111542061
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/OperationsResource.scala:
##########
@@ -183,19 +183,47 @@ private[v1] class OperationsResource extends ApiRequestContext with Logging {
i.getSetField.name(),
i.getSetField match {
case TColumnValue._Fields.STRING_VAL =>
- i.getStringVal.getFieldValue(TStringValue._Fields.VALUE)
+ if (i.getStringVal.isSetValue) {
Review Comment:
A possible risk is that `getStringVal` on the unset field will throw RuntimeException here.
Maybe use `i.isSetStringVal` for the case condition is better? And make null as default return.
--
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