[common][spark] Fix boolean hilbert value colliding with the null sentinel - #9515
Open
LuciferYang wants to merge 1 commit into
Open
[common][spark] Fix boolean hilbert value colliding with the null sentinel#9515LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
…tinel HilbertIndexer's BOOLEAN visitor and its Spark twin SparkHilbertUDF.booleanToOrderedLongUDF both mapped TRUE to PRIMITIVE_EMPTY (Long.MAX_VALUE), the value every other type uses for null, so a TRUE value and a NULL value landed on the same point of the curve. Map TRUE to 1 and keep FALSE at 0 on both sides. The Spark UDF also lacked the null guard its eight siblings have, so a nullable boolean order column unboxed into a NullPointerException on the executor. apache#7451 added that guard to the string and binary UDFs and left boolean out. Assisted-by: GLM-5.3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
close #9514
Hilbert clustering maps each order column to a long before placing it on the curve, and
PRIMITIVE_EMPTY(Long.MAX_VALUE) is the value every type uses for null. The boolean mapping used it for TRUE, in both copies of the mapping:HilbertIndexerin paimon-common andSparkHilbertUDF.booleanToOrderedLongUDFin paimon-spark-common. A TRUE value and a NULL value therefore landed on the same point of the curve, so rows that differ on that column clustered as if they were equal, and aa = truepredicate lost the file skipping the clustering was supposed to buy. TRUE now maps to 1 and FALSE stays at 0 on both sides.The Spark UDF was also the only one of the nine in that file without a null check.
functions.udf((Boolean value) -> ...)has no input encoders, so Spark'sHandleNullInputsForUDFdoes not insert a null short circuit and the lambda receivesnulldirectly, wherevalue ?unboxed it into aNullPointerExceptionon the executor. Clustering a nullable boolean column throughsys.compactfailed outright. #7451 added this guard to the string and binary UDFs in the same two files and left boolean out.The hilbert value is a transient sort key:
HilbertSorteradds it, sorts on it and drops it, on both engines. Nothing on disk records it, so existing tables read back unchanged and only the layout produced by a future sort-compact or clustered write differs.Tests
HilbertIndexerTest.testBooleanValuesDistinctFromNull(new file) asserts the curve position of a FALSE row, a TRUE row and a NULL row againsthilbertCurvePosBytesof{0, 0},{1, 1}and{MAX_VALUE, MAX_VALUE}. Pinning the exact positions pins the mapping, so an inverted mapping that keeps the three distinct cannot pass and drift away from the Spark side; the three pairwise-distinct assertions are kept as an explicit statement of the collision itself.SparkHilbertUDFTest.testBooleanColumnMapsNullFalseAndTrueToDistinctValues(new file) runs a localSparkSessionover a nullable BOOLEAN column with true, false and null rows throughsortedLexicographically(col, BooleanType)and asserts null maps toLong.MAX_VALUE, TRUE to 1 and FALSE to 0. It lives in paimon-spark-common next toSortedIndexTopoBuilderTest, which already builds a local session there, and runs in about three seconds.Verified red before the change: the indexer test fails on the exact-position assertion, and the Spark test fails with
java.lang.NullPointerException, which also confirms that Spark really does handnullto that lambda.mvn -pl paimon-common teston JDK 8: 12466 tests, 0 failures.mvn -pl paimon-spark/paimon-spark-common -Dtest='SparkHilbertUDFTest,SortedIndexTopoBuilderTest' test: 5 tests, 0 failures, both session-creating classes green in one JVM. checkstyle, spotless, enforcer and rat run clean on both modules.