You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "qindongliang (via GitHub)" <gi...@apache.org> on 2025/12/18 15:00:33 UTC

[PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

qindongliang opened a new pull request, #53529:
URL: https://github.com/apache/spark/pull/53529

   ### What changes were proposed in this pull request?
   
   Fixed a bug in `RoundBase` where `ROUND` function incorrectly returns `NULL` for certain Decimal values. The issue was caused by using the input decimal's runtime precision instead of the target type's precision when calling `toPrecision()`.
   
   For example, `ROUND(PERCENTILE_APPROX(2150 / 1000.0, 0.95), 3)` incorrectly returned `NULL` instead of `2.15`.
   
   ### Why are the changes needed?
   
   The fix changes `decimal.toPrecision(decimal.precision, s, mode)` to `decimal.toPrecision(p, s, mode)` in both `nullSafeEval` and `doGenCode` methods, where `p` is the target DecimalType's precision.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, this fixes a bug where ROUND could return NULL for valid decimal inputs.
   
   ### How was this patch tested?
   
   Added regression test in `ApproximatePercentileQuerySuite`.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "qindongliang (via GitHub)" <gi...@apache.org>.
qindongliang commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3673671737

   Hi @yaooqinn   @gengliangwang 
   The CI failure in "Base image build" is expected for external contributors due to GitHub's security restrictions on GHCR package writes from forked PRs.
   Could you please help trigger the official CI when convenient? Thanks!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "qindongliang (via GitHub)" <gi...@apache.org>.
qindongliang commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675306225

   > @qindongliang, can you make the GA green? I speculate it is due to your GitHub settings.
   
   Hi @yaooqinn,
   
   Thank you for triggering the CI. I investigated the failure and found the root cause:
   
   The build itself succeeded (all layers are cached from Apache's registry), but the push 
   to `ghcr.io/qindongliang/apache-spark-ci-image-docs:...` failed with `permission_denied: write_package`.
   
   This happens because:
   1. The workflow uses `${{ github.repository_owner }}` to generate the image URL, 
      which becomes `ghcr.io/qindongliang/...` in my fork
   2. GitHub doesn't allow Actions to automatically create new packages in the Container Registry
   3. I've already set "Read and write permissions" for workflows in my repo settings, 
      but it still doesn't work for creating new packages
   
   My questions:
   1. Is there a recommended way to configure fork repos to allow pushing to ghcr.io? 
      Should I manually create placeholder packages first?
   2. Or could you run the CI directly from the apache/spark side where the packages 
      already exist?
   
   Any guidance would be appreciated. Thank you!
   <img width="2204" height="758" alt="image" src="https://github.com/user-attachments/assets/29048355-b245-4af8-95a4-74a1806e808e" />
   
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "qlong (via GitHub)" <gi...@apache.org>.
qlong commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675421662

   LGTM


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "qindongliang (via GitHub)" <gi...@apache.org>.
qindongliang commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3670760285

   Jira: https://issues.apache.org/jira/browse/SPARK-54750


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn closed pull request #53529: [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l…
URL: https://github.com/apache/spark/pull/53529


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "qindongliang (via GitHub)" <gi...@apache.org>.
qindongliang commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675451263

   Thanks @yaooqinn! @qlong 
   
   I found the issue - I needed to grant my home repo write access to the packages in GitHub Container Registry settings. 
   
   The image push is now successful. CI is currently building and should turn green soon if everything goes smoothly. ✅
   
   Reference: https://github.com/orgs/community/discussions/32184


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675429565

   I'm not familiar with this area either, maybe you can check https://github.com/qindongliang?tab=packages for what is missing


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675762499

   Merged to master and '4.1.1', '4.0.2', '3.5.8'


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54750][SQL] Fix ROUND returning NULL for Decimal values with l… [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #53529:
URL: https://github.com/apache/spark/pull/53529#issuecomment-3675010229

   @qindongliang, can you make the GA green? I speculate it is due to your GitHub settings.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org