Skip to content

Add SQL input sanitization utilities - #19352

Merged
xiangfu0 merged 1 commit into
apache:masterfrom
xiangfu0:codex/sql-identifier-utils
Aug 25, 2026
Merged

Add SQL input sanitization utilities#19352
xiangfu0 merged 1 commit into
apache:masterfrom
xiangfu0:codex/sql-identifier-utils

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Adds shared ParserUtils helpers for safely constructing SQL from dynamic inputs:

  • sanitizeIdentifier preserves valid bare identifiers and wildcards, normalizes quoted qualified names, and quotes unsafe components
  • sanitizeAggregationFunction accepts only registered Pinot aggregation functions
  • sanitizePredicate rejects statement separators/comments outside quoted content and requires the complete predicate to compile as a Pinot SQL expression, while allowing valid right-hand-side column references

The identifier path avoids qualified-name allocation for the common unquoted case. These utilities let downstream query builders, including startreedata/startree-pinot#4093, delegate validation instead of maintaining parallel security logic.

Validation:

  • ParserUtilsTest: 64 tests, 0 failures
  • pinot-common spotless, license, and checkstyle checks
  • pinot-common test compilation with JDK 25
  • git diff --check

@xiangfu0 xiangfu0 added the feature New functionality label Aug 24, 2026
Comment thread pinot-common/src/main/java/org/apache/pinot/sql/parsers/ParserUtils.java Outdated
@codecov-commenter

codecov-commenter commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.85965% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.14%. Comparing base (7991438) to head (93213e6).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...java/org/apache/pinot/sql/parsers/ParserUtils.java 93.85% 1 Missing and 6 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19352      +/-   ##
============================================
+ Coverage     67.13%   67.14%   +0.01%     
  Complexity     1424     1424              
============================================
  Files          3468     3468              
  Lines        222432   222546     +114     
  Branches      35015    35048      +33     
============================================
+ Hits         149329   149439     +110     
+ Misses        61200    61192       -8     
- Partials      11903    11915      +12     
Flag Coverage Δ
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 67.14% <93.85%> (+0.01%) ⬆️
lane-a 100.00% <ø> (ø)
lane-b 0.00% <ø> (ø)
temurin 67.14% <93.85%> (+0.01%) ⬆️
unittests 67.14% <93.85%> (+0.01%) ⬆️
unittests1 57.88% <93.85%> (+<0.01%) ⬆️
unittests2 38.87% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0
xiangfu0 force-pushed the codex/sql-identifier-utils branch from c7691e1 to 957ede8 Compare August 24, 2026 21:36
@xiangfu0 xiangfu0 changed the title Add qualified SQL identifier sanitization utility Add SQL input sanitization utilities Aug 24, 2026
Centralize parser-backed validation for dynamically generated SQL so callers share identifier quoting, aggregation validation, and predicate checks.

This lets downstream query builders such as StarTree QueryGenerator delegate validation instead of maintaining parallel security logic.
@xiangfu0
xiangfu0 force-pushed the codex/sql-identifier-utils branch from 957ede8 to 93213e6 Compare August 24, 2026 23:28
@xiangfu0
xiangfu0 merged commit 1961c00 into apache:master Aug 25, 2026
12 checks passed
@xiangfu0
xiangfu0 deleted the codex/sql-identifier-utils branch August 25, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants