Skip to content

test: add SQL file tests for aes_encrypt, aes_decrypt, try_aes_decrypt#4557

Open
andygrove wants to merge 3 commits into
apache:mainfrom
andygrove:test-aes-coverage
Open

test: add SQL file tests for aes_encrypt, aes_decrypt, try_aes_decrypt#4557
andygrove wants to merge 3 commits into
apache:mainfrom
andygrove:test-aes-coverage

Conversation

@andygrove
Copy link
Copy Markdown
Member

@andygrove andygrove commented Jun 1, 2026

Which issue does this PR close?

N/A. This adds test coverage only. Related: #4558.

Rationale for this change

The AES functions (aes_encrypt, aes_decrypt, try_aes_decrypt) had no Comet test coverage. They are RuntimeReplaceable and lower to a StaticInvoke of ExpressionImplUtils.aesEncrypt / aesDecrypt. This test pins down their current behavior in Comet.

What changes are included in this PR?

Adds spark/src/test/resources/sql-tests/expressions/misc/aes.sql, covering GCM (default), ECB, and CBC modes; 16/24/32-byte keys; column and literal arguments; NULL inputs; and try_aes_decrypt on invalid ciphertext. Because aes_encrypt uses a random IV for GCM/CBC (nondeterministic output), those cases are tested via round-trip (aes_decrypt(aes_encrypt(x, k), k) = x); ECB output is deterministic and compared directly.

Empirical finding: all three functions currently fall back to Spark. Comet's StaticInvoke serde only allowlists a few target methods (readSidePadding, isLuhnNumber, URL encode/decode), and aesEncrypt / aesDecrypt are not among them, so the plan reports Static invoke expression: aesEncrypt is not supported. All queries therefore use query spark_answer_only (results are still validated against Spark). When native support is added, they can be promoted to the default query mode.

How are these changes tested?

This PR is test-only. The file runs under CometSqlFileTestSuite. Verified locally:

./mvnw test -Dsuites="org.apache.comet.CometSqlFileTestSuite aes" -Dtest=none

Result: the discovered test file passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant