You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "davidhcoe (via GitHub)" <gi...@apache.org> on 2023/10/31 20:21:37 UTC
[I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
davidhcoe opened a new issue, #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242
We are still having issues with decimals even with high precision enabled. Consider the following scenarios:
```
CREATE OR REPLACE TABLE PUBLIC.NUMBERTEST_ALLPRECISION1
(
COL18 NUMBER(18,0),COL19 NUMBER(19,0),COL20 NUMBER(20,0),COL21 NUMBER(21,0),COL22 NUMBER(22,0),COL23 NUMBER(23,0),COL24 NUMBER(24,0),COL25 NUMBER(25,0),COL26 NUMBER(26,0),COL27 NUMBER(27,0),COL28 NUMBER(28,0),COL29 NUMBER(29,0),COL30 NUMBER(30,0),COL31 NUMBER(31,0),COL32 NUMBER(32,0),COL33 NUMBER(33,0),COL34 NUMBER(34,0),COL35 NUMBER(35,0),COL36 NUMBER(36,0),COL37 NUMBER(37,0),COL38 NUMBER(38,0)
);
INSERT INTO NUMBERTEST_ALLPRECISION1
(
COL18,COL19, COL20, COL21, COL22, COL23, COL24, COL25, COL26, COL27, COL28, COL29, COL30, COL31, COL32, COL33, COL34, COL35, COL36, COL37, COL38
)
VALUES
(
999999999999999999, 9999999999999999999, 99999999999999999999, 999999999999999999999, 9999999999999999999999, 99999999999999999999999, 999999999999999999999999, 9999999999999999999999999, 99999999999999999999999999, 999999999999999999999999999, 9999999999999999999999999999, 99999999999999999999999999999, 999999999999999999999999999999, 9999999999999999999999999999999, 99999999999999999999999999999999, 999999999999999999999999999999999, 9999999999999999999999999999999999, 99999999999999999999999999999999999, 999999999999999999999999999999999999, 9999999999999999999999999999999999999,99999999999999999999999999999999999999
);
```
When this table is queried, the error from https://github.com/apache/arrow/blob/2628d495ca99a892dca50019f4e72f087dc5aac7/go/arrow/compute/internal/kernels/numeric_cast.go#L230 is hit, because COL18 has a precision+scale value of less than 19 (per https://github.com/apache/arrow/blob/c49e24273160ac1ce195f02dbd14acd7d0f6945e/go/arrow/compute/internal/kernels/helpers.go#L701).
The other scenario is:
```
CREATE OR REPLACE TABLE PUBLIC.NUMBERTEST_ALLPRECISION1
(
COL18 NUMBER(18,2),COL19 NUMBER(19,2),COL20 NUMBER(20,2),COL21 NUMBER(21,2),COL22 NUMBER(22,02),COL23 NUMBER(23,02),COL24 NUMBER(24,02),COL25 NUMBER(25,02),COL26 NUMBER(26,02),COL27 NUMBER(27,02),COL28 NUMBER(28,02),COL29 NUMBER(29,02),COL30 NUMBER(30,02),COL31 NUMBER(31,02),COL32 NUMBER(32,02),COL33 NUMBER(33,02),COL34 NUMBER(34,02),COL35 NUMBER(35,02),COL36 NUMBER(36,02),COL37 NUMBER(37,02),COL38 NUMBER(38,02)
);
INSERT INTO NUMBERTEST_ALLPRECISION1
(
COL18,COL19, COL20, COL21, COL22, COL23, COL24, COL25, COL26, COL27, COL28, COL29, COL30, COL31, COL32, COL33, COL34, COL35, COL36, COL37, COL38
)
VALUES
(
9999999999999999.99, 99999999999999999.99, 999999999999999999.99, 9999999999999999999.99, 99999999999999999999.99, 999999999999999999999.99, 9999999999999999999999.99, 99999999999999999999999.99, 999999999999999999999999.99, 9999999999999999999999999.99, 99999999999999999999999999.99, 999999999999999999999999999.99, 9999999999999999999999999999.99, 99999999999999999999999999999.99, 999999999999999999999999999999.99, 9999999999999999999999999999999.99, 99999999999999999999999999999999.99, 999999999999999999999999999999999.99, 9999999999999999999999999999999999.99, 99999999999999999999999999999999999.99,999999999999999999999999999999999999.99
);
```
When this table is queried, https://github.com/apache/arrow/blob/c49e24273160ac1ce195f02dbd14acd7d0f6945e/go/arrow/compute/internal/kernels/helpers.go#L592 is hit and cannot return values.
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1789736124
@zeroshade would these be primarily upstream issues?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1794817410
I'm currently travelling for a conference and unable to dedicate time to attempting to fix this directly myself. But if you want to take a stab at addressing it I'll happily review the code next week!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "davidhcoe (via GitHub)" <gi...@apache.org>.
davidhcoe commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1790905215
> In the case of hitting https://github.com/apache/arrow/blob/c49e24273160ac1ce195f02dbd14acd7d0f6945e/go/arrow/compute/internal/kernels/helpers.go#L592 it should only hit that if we're actually hitting the case where the value itself falls outside the range (since it should only error by actually _checking_ that the value didn't fit), but for the early failure due to the scale and precision I guess we can add an option upstream to allow a consumer to request it to bypass that validation and let it just assume that everything will fit?
>
> I'm not sure how I feel about that and I wish that snowflake would just return the correct type in the first place, but I doubt we'd be able to convince them to change how they return the data....
Right. I tried adding logic to check whether `useHighPrecision` could be used given the precision and scale, and the current answer is false, so it falls to the logic of trying to do the math on the larger values that don't fit. So that didn't turn out to be a good idea.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1792744006
There's a fair amount of logic in https://github.com/snowflakedb/gosnowflake/blob/master/converter.go which simply isn't reflected in the ADBC driver. Specifically, Snowflake will use lower-precision integer types to return lower-precision decimal values e.g. a decimal(9, 2) could be returned as an Arrow int32 and would need to be widened and not just converted as-is. This affects both the high-precision and low-precision code paths.
I can take a look at addressing this, but it will be the first time I've written Go ;).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1789752539
In at least one case, the column is typed as NUMBER(18, 2) and the array returned from Snowflake is typed as INT64. Obviously, INT64 values aren't guaranteed to fit into a NUMBER(18, 2) so the conversion code fails early. But it happens that all the values returned from Snowflake in that array do actually fit.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm closed issue #1242: go/adbc/driver/snowflake: improved support for decimal128 types
URL: https://github.com/apache/arrow-adbc/issues/1242
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1794827625
I've been looking at this intermittently over the weekend and trying to pick up some Go by looking at a simpler issue first. I know what needs to be done mechanically, but I'm still trying to figure out how the compute kernels work and whether I can pass a custom function instead of relying on a predefined kernel as I think that the functionality this needs is a bit too specialized to justify putting into Arrow itself (and don't want to wait for a new Arrow release).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1790884577
In the case of hitting https://github.com/apache/arrow/blob/c49e24273160ac1ce195f02dbd14acd7d0f6945e/go/arrow/compute/internal/kernels/helpers.go#L592 it should only hit that if we're actually hitting the case where the value itself falls outside the range (since it should only error by actually *checking* that the value didn't fit), but for the early failure due to the scale and precision I guess we can add an option upstream to allow a consumer to request it to bypass that validation and let it just assume that everything will fit?
I'm not sure how I feel about that and I wish that snowflake would just return the correct type in the first place, but I doubt we'd be able to convince them to change how they return the data....
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] go/adbc/driver/snowflake: improved support for decimal128 types [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #1242:
URL: https://github.com/apache/arrow-adbc/issues/1242#issuecomment-1795002338
In the Arrow Documentation https://pkg.go.dev/github.com/apache/arrow/go/v14@v14.0.0/arrow/compute#example-package-CustomFunction provides an example on constructing, registering, and calling a custom function through the compute package. That should help, feel free to reach out to me if you need any more assistance.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org