You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/12 21:22:41 UTC
[GitHub] [pinot] Jackie-Jiang opened a new pull request, #9385: Do not allow implicit cast between number, string, binary
Jackie-Jiang opened a new pull request, #9385:
URL: https://github.com/apache/pinot/pull/9385
In standard sql, implicit cast between number, string, binary is not allowed. Trying to support it can lead to ambiguous semantic and unnecessary overhead. We have observed performance regression after #9287.
This PR removes the implicit cast between number, string and binary. The explicit conversion is still supported with `CAST` transform function.
## Backward Incompatible
Implicit cast between number, string, binary is no longer allowed. Use `CAST` if the conversion is needed.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9385:
URL: https://github.com/apache/pinot/pull/9385#discussion_r970931836
##########
pinot-core/src/test/java/org/apache/pinot/core/common/DataFetcherTest.java:
##########
@@ -221,45 +204,23 @@ public void testFetchBigDecimalValues() {
testFetchBigDecimalValues(FLOAT_COLUMN);
testFetchBigDecimalValues(DOUBLE_COLUMN);
testFetchBigDecimalValues(BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(STRING_COLUMN);
testFetchBigDecimalValues(NO_DICT_INT_COLUMN);
testFetchBigDecimalValues(NO_DICT_LONG_COLUMN);
testFetchBigDecimalValues(NO_DICT_FLOAT_COLUMN);
testFetchBigDecimalValues(NO_DICT_DOUBLE_COLUMN);
testFetchBigDecimalValues(NO_DICT_BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(NO_DICT_STRING_COLUMN);
}
@Test
public void testFetchStringValues() {
- testFetchStringValues(INT_COLUMN);
- testFetchStringValues(LONG_COLUMN);
- testFetchStringValues(FLOAT_COLUMN);
- testFetchStringValues(DOUBLE_COLUMN);
- testFetchStringValues(BIG_DECIMAL_COLUMN);
testFetchStringValues(STRING_COLUMN);
- testFetchStringValues(NO_DICT_INT_COLUMN);
- testFetchStringValues(NO_DICT_LONG_COLUMN);
- testFetchStringValues(NO_DICT_FLOAT_COLUMN);
- testFetchStringValues(NO_DICT_DOUBLE_COLUMN);
- testFetchStringValues(NO_DICT_BIG_DECIMAL_COLUMN);
Review Comment:
sounds good. good to add release note and backward incompatible tags to explain
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9385:
URL: https://github.com/apache/pinot/pull/9385#discussion_r970277319
##########
pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java:
##########
@@ -49,11 +49,7 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertThrows;
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.*;
Review Comment:
We usually import `Assert.*` in test to simplify the code
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9385: Do not allow implicit cast for BOOLEAN and TIMESTAMP
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9385:
URL: https://github.com/apache/pinot/pull/9385#issuecomment-1249947958
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9385?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 [#9385](https://codecov.io/gh/apache/pinot/pull/9385?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (76ba24f) into [master](https://codecov.io/gh/apache/pinot/commit/2d6665b8e5fa0842ef67b3d9896c5e04ecad78e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d6665b) will **decrease** coverage by `6.19%`.
> The diff coverage is `58.25%`.
```diff
@@ Coverage Diff @@
## master #9385 +/- ##
============================================
- Coverage 69.73% 63.54% -6.20%
- Complexity 4787 5072 +285
============================================
Files 1890 1837 -53
Lines 100756 98549 -2207
Branches 15350 15066 -284
============================================
- Hits 70266 62625 -7641
- Misses 25507 31313 +5806
+ Partials 4983 4611 -372
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.07% <58.25%> (+0.06%)` | :arrow_up: |
| unittests2 | `15.35% <0.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9385?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/function/scalar/DataTypeConversionFunctions.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0RhdGFUeXBlQ29udmVyc2lvbkZ1bmN0aW9ucy5qYXZh) | `52.38% <ø> (ø)` | |
| [...ator/transform/function/CaseTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `54.58% <0.00%> (-7.25%)` | :arrow_down: |
| [...main/java/org/apache/pinot/spi/data/FieldSpec.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9GaWVsZFNwZWMuamF2YQ==) | `81.33% <ø> (ø)` | |
| [...r/transform/function/ValueInTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVmFsdWVJblRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `81.81% <20.00%> (ø)` | |
| [...ava/org/apache/pinot/spi/utils/ArrayCopyUtils.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQXJyYXlDb3B5VXRpbHMuamF2YQ==) | `68.36% <43.85%> (+4.40%)` | :arrow_up: |
| [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `75.77% <50.81%> (+4.72%)` | :arrow_up: |
| [...ator/transform/function/CastTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQ2FzdFRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `57.00% <55.26%> (-19.92%)` | :arrow_down: |
| [...or/transform/function/LookupTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTG9va3VwVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `77.10% <75.67%> (-2.07%)` | :arrow_down: |
| [...java/org/apache/pinot/core/common/DataFetcher.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUZldGNoZXIuamF2YQ==) | `88.97% <84.61%> (+11.15%)` | :arrow_up: |
| [...orm/function/LogicalOperatorTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/9385/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTG9naWNhbE9wZXJhdG9yVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `95.83% <100.00%> (+4.52%)` | :arrow_up: |
| ... and [436 more](https://codecov.io/gh/apache/pinot/pull/9385/diff?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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9385:
URL: https://github.com/apache/pinot/pull/9385#discussion_r970276123
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java:
##########
@@ -43,6 +43,7 @@ private DataTypeConversionFunctions() {
public static Object cast(Object value, String targetTypeLiteral) {
try {
Class<?> clazz = value.getClass();
+ // TODO: Support cast for MV
Review Comment:
Yes. It is supported in the transform function, but not in the scalar function
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #9385: Do not allow implicit cast for BOOLEAN and TIMESTAMP
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #9385:
URL: https://github.com/apache/pinot/pull/9385
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9385:
URL: https://github.com/apache/pinot/pull/9385#discussion_r969889195
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java:
##########
@@ -43,6 +43,7 @@ private DataTypeConversionFunctions() {
public static Object cast(Object value, String targetTypeLiteral) {
try {
Class<?> clazz = value.getClass();
+ // TODO: Support cast for MV
Review Comment:
did you meant element-wise? how would the semantic work?
##########
pinot-core/src/test/java/org/apache/pinot/queries/MultiValueRawQueriesTest.java:
##########
@@ -49,11 +49,7 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertThrows;
-import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.*;
Review Comment:
import Assert instead of star import?
##########
pinot-core/src/test/java/org/apache/pinot/core/common/DataFetcherTest.java:
##########
@@ -221,45 +204,23 @@ public void testFetchBigDecimalValues() {
testFetchBigDecimalValues(FLOAT_COLUMN);
testFetchBigDecimalValues(DOUBLE_COLUMN);
testFetchBigDecimalValues(BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(STRING_COLUMN);
testFetchBigDecimalValues(NO_DICT_INT_COLUMN);
testFetchBigDecimalValues(NO_DICT_LONG_COLUMN);
testFetchBigDecimalValues(NO_DICT_FLOAT_COLUMN);
testFetchBigDecimalValues(NO_DICT_DOUBLE_COLUMN);
testFetchBigDecimalValues(NO_DICT_BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(NO_DICT_STRING_COLUMN);
}
@Test
public void testFetchStringValues() {
- testFetchStringValues(INT_COLUMN);
- testFetchStringValues(LONG_COLUMN);
- testFetchStringValues(FLOAT_COLUMN);
- testFetchStringValues(DOUBLE_COLUMN);
- testFetchStringValues(BIG_DECIMAL_COLUMN);
testFetchStringValues(STRING_COLUMN);
- testFetchStringValues(NO_DICT_INT_COLUMN);
- testFetchStringValues(NO_DICT_LONG_COLUMN);
- testFetchStringValues(NO_DICT_FLOAT_COLUMN);
- testFetchStringValues(NO_DICT_DOUBLE_COLUMN);
- testFetchStringValues(NO_DICT_BIG_DECIMAL_COLUMN);
Review Comment:
I understand implicit type cast from STRING to NUMBER is not a good practice. but do we want to support the otherway around?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9385:
URL: https://github.com/apache/pinot/pull/9385#discussion_r970277001
##########
pinot-core/src/test/java/org/apache/pinot/core/common/DataFetcherTest.java:
##########
@@ -221,45 +204,23 @@ public void testFetchBigDecimalValues() {
testFetchBigDecimalValues(FLOAT_COLUMN);
testFetchBigDecimalValues(DOUBLE_COLUMN);
testFetchBigDecimalValues(BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(STRING_COLUMN);
testFetchBigDecimalValues(NO_DICT_INT_COLUMN);
testFetchBigDecimalValues(NO_DICT_LONG_COLUMN);
testFetchBigDecimalValues(NO_DICT_FLOAT_COLUMN);
testFetchBigDecimalValues(NO_DICT_DOUBLE_COLUMN);
testFetchBigDecimalValues(NO_DICT_BIG_DECIMAL_COLUMN);
- testFetchBigDecimalValues(NO_DICT_STRING_COLUMN);
}
@Test
public void testFetchStringValues() {
- testFetchStringValues(INT_COLUMN);
- testFetchStringValues(LONG_COLUMN);
- testFetchStringValues(FLOAT_COLUMN);
- testFetchStringValues(DOUBLE_COLUMN);
- testFetchStringValues(BIG_DECIMAL_COLUMN);
testFetchStringValues(STRING_COLUMN);
- testFetchStringValues(NO_DICT_INT_COLUMN);
- testFetchStringValues(NO_DICT_LONG_COLUMN);
- testFetchStringValues(NO_DICT_FLOAT_COLUMN);
- testFetchStringValues(NO_DICT_DOUBLE_COLUMN);
- testFetchStringValues(NO_DICT_BIG_DECIMAL_COLUMN);
Review Comment:
I don't think implicit type cast from other type to STRING is good practice either, and we have observed performance regression doing it in #9287
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #9385: Do not allow implicit cast for BOOLEAN and TIMESTAMP
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #9385:
URL: https://github.com/apache/pinot/pull/9385#issuecomment-1249938124
After some discussion, decided to keep the existing supported conversions, but revert the new added #9287 which causes the performance regression.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9385:
URL: https://github.com/apache/pinot/pull/9385#issuecomment-1247391278
> Is there any reference available that talks about the restriction on implicit cast between them? If yes, can you link it to the description section of this PR?
>
> We might need to carefully monitor the deployment in order to detect any existing use cases once this PR is merged.
yes I was suggesting either a release note or a backward incompatible section in the PR but also in Pinot doc.
In terms of the rationale, AFAIK,
- nothing should be implicitly cast in logical plan. what we do NOW is b/c:
- if a function expects `foo(int, long)` then they data type should be exact
- however many system implements overload `foo(int, int), foo(int, long), foo(long, long) ...` so it will always find one properly. Pinot doesn't support these type of operator/function overloading thus we need to provide implicit cast.
- many logical planner insert explicit CAST operator if the type doesn't match in user's provided SQL, for example:
- `SELECT intCol + strCol FROM tbl` will result in a
```
LogicalProject(
[+(intCol, CAST(strCol, INT))],
LogicalScan(
tbl,
[intCol, strCol]
)
)
```
so IMO as long as we state clearly in our user-facing implicit cast rule in doc we are good. I am incline to the following:
- all numeric should have implicit cast and (1) throw exception (2) put MAX/MIN_VAL, or NaN when overflow
- all DataType sharing the same StorageType should be able to implicitly cast between (for example JSON / STRING)
- all other should not have default implicit cast rule
-
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #9385: Do not allow implicit cast between number, string, binary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #9385:
URL: https://github.com/apache/pinot/pull/9385#issuecomment-1247415824
@walterddr @jackjlli Seems different DB have different implicit conversion behavior between number and string:
Presto: do not allow - https://prestodb.io/docs/current/functions/conversion.html
MySQL: allow - https://dev.mysql.com/doc/refman/8.0/en/type-conversion.html
SQL Server: allow - https://docs.microsoft.com/en-us/sql/t-sql/data-types/data-type-conversion-database-engine?view=sql-server-ver16
Let me see if we can support it without paying too much overhead
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org