test(spark): enhance blob/vector SQL DDL coverage of the extended parser - #19408
Draft
yihua wants to merge 2 commits into
Draft
test(spark): enhance blob/vector SQL DDL coverage of the extended parser#19408yihua wants to merge 2 commits into
yihua wants to merge 2 commits into
Conversation
Add parser-coverage cases to the existing blob/vector SQL DDL and index syntax suites so the per-version extended AST builder's CREATE TABLE clause visitors are exercised. The extended parser is reached only when a statement contains a blob or vector column, so every new case keeps such a column and asserts the parsed CreateTable plan (schema and partitioning) or the exact parse error. Covers the data-type arms, partition transforms and their literal arguments, bucket spec, file and row format clauses, table properties and options, and the previously untested index DDL branches. Test only; no production code changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19408 +/- ##
============================================
+ Coverage 74.83% 76.07% +1.24%
- Complexity 32332 33319 +987
============================================
Files 2574 2574
Lines 142978 142991 +13
Branches 17527 17555 +28
============================================
+ Hits 106992 108781 +1789
+ Misses 27921 25891 -2030
- Partials 8065 8319 +254
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…x extended parser The Spark 4.x extended parser rejects a bare TIMESTAMP token (column type or typed literal) in a blob/vector CREATE TABLE, while spark3.5 accepts it. Retype the transform-source columns to DATE, drop the standalone TIMESTAMP primitive column, and use a DATE typed-literal constructor so the cases parse on all six profiles.
Collaborator
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
3 participants
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.
Describe the issue this Pull Request addresses
The six per-version extended SQL AST builders (
HoodieSpark{3.3,3.4,3.5,4.0,4.1,4.2}ExtendedSqlAstBuilder.scala) were only about a quarter line-covered. The extended parser is reached only when a statement contains a Hudi-specificbloborvectortoken (isHoodieCommand), and such a statement is then parsed entirely by this builder. After the recent prune of the forked parser to Hudi-only statements (#19132), the remaining CREATE TABLE clause visitors (partition transforms, literal arguments, bucket spec, file and row format, interval and other data types, table properties and options) are all reachable and required, but were largely unexercised because the existing tests used a bareblob/vectorcolumn with no other clauses.Summary and Changelog
Enhances the existing blob/vector SQL DDL and index syntax test suites to exercise the extended AST builder's CREATE TABLE clause visitors. This is a test-only, parser-coverage change. Because the builder is not dead code (every remaining path is reachable through a blob/vector statement), no further prune is warranted.
New cases route through the extended builder by keeping a BLOB or VECTOR column in every statement, and assert the parsed logical plan (
CreateTable.tableSchemaandCreateTable.partitioning, both stable across Spark 3.3 through 4.2) viaspark.sessionState.sqlParser.parsePlan, or assert the exact parse error for the negative cases. Parse-level assertions exercise the builder without needing table creation, so they also cover clauses Hudi does not support at execution time (transform partitioning, STORED AS / ROW FORMAT, interval columns).Suites enhanced (no new suite added):
TestBlobDataType: primitive, complex and interval data-type arms; partition transforms (identity, years/months/days/hours, generic apply, and bucket with int/long/short counts); typed transform-argument literals (string, boolean, null, integer, timestamp constructor, multi-unit interval); their error twins (invalid bucket count, non-column reference, too many arguments, reversed interval endpoints, unknown type); and file-format / row-format clauses (STORED AS generic and INPUTFORMAT/OUTPUTFORMAT, ROW FORMAT SERDE / DELIMITED, plus the incompatible-combination, STORED BY, and USING-with-serde errors).TestCreateTable(vector): partition transforms; typed transform-argument literals (adds long, exponent/double, and unit-to-unit interval); CLUSTERED BY bucket spec (plain, SORTED BY ASC, and the SORTED BY DESC rejection); and LOCATION / COMMENT / OPTIONS / TBLPROPERTIES with string, integer and boolean values, plus the path-collision and reserved-property (provider/location/owner) errors.TestIndexSyntax: folds the uncovered index branches into the existing fixture-backed test: CREATE INDEX without USING (default empty index type), DROP INDEX without IF EXISTS, and SHOW INDEXES with the IN keyword.Findings from writing these tests:
TIMESTAMP-typed column, and aTIMESTAMP '...'typed literal, fail to parse in the Spark 4.x extended parser for a blob/vector CREATE TABLE ("no viable alternative at input 'TIMESTAMP'"), though the Spark 3.5 extended parser accepts both. The tests useDATEinstead. This looks like a Spark 4 extended-parser gap worth a separate look.qualifiedNamealternative takes precedence over aconstant, and under the default non-ANSI configtrue/false/nullare valid identifiers. SovisitBooleanLiteralandvisitNullLiteralare not reachable from a CREATE TABLE statement and are left unasserted.getSingleFieldReference's empty-arguments branch ("Not enough arguments for transform") is unreachable from SQL and is not asserted.Impact
None. Test-only change; no production code or public API is modified.
Risk Level
none
Documentation Update
none
Contributor's checklist