Skip to content

Add SQL DDL support for CREATE/DROP TABLE and SHOW CREATE TABLE#18241

Merged
xiangfu0 merged 32 commits into
apache:masterfrom
xiangfu0:feature/sql-ddl-slices-1-2
May 16, 2026
Merged

Add SQL DDL support for CREATE/DROP TABLE and SHOW CREATE TABLE#18241
xiangfu0 merged 32 commits into
apache:masterfrom
xiangfu0:feature/sql-ddl-slices-1-2

Conversation

@xiangfu0
Copy link
Copy Markdown
Contributor

@xiangfu0 xiangfu0 commented Apr 16, 2026

Summary

Adds a new SQL DDL surface to Pinot — CREATE TABLE, DROP TABLE, SHOW TABLES, SHOW CREATE TABLE — exposed via a single controller endpoint POST /sql/ddl. DDL statements compile into the existing Schema + TableConfig pair and persist through the same PinotHelixResourceManager paths as the JSON /tables and /schemas endpoints, sharing the validation pipeline (TableConfigValidationUtils) so DDL-created tables are subject to the same min-replica, storage-quota, hybrid-pair, instance-assignment, and tenant-tag checks as JSON-API tables.

The implementation is split into three layers:

  • Parser (pinot-common): Calcite FMPP/JavaCC grammar additions + 6 AST node classes.
  • Compiler (pinot-sql-ddl — new module): DdlCompiler translates AST → (Schema, TableConfig); CanonicalDdlEmitter does the reverse for SHOW CREATE TABLE. Round-trip is idempotent.
  • REST endpoint (pinot-controller): POST /sql/ddl dispatches by operation, applies tuner configs before validation, supports dryRun=true for compile-without-persist.

Quickstart examples

Create an OFFLINE table

CREATE TABLE events (
  id    INT NOT NULL DIMENSION,
  city  STRING DIMENSION,
  amount DOUBLE METRIC,
  ts    TIMESTAMP DATETIME FORMAT 'TIMESTAMP' GRANULARITY '1:MILLISECONDS'
)
TABLE_TYPE = OFFLINE
PROPERTIES (
  'timeColumnName' = 'ts',
  'replication' = '3',
  'brokerTenant' = 'DefaultTenant',
  'serverTenant' = 'DefaultTenant'
);

TIMESTAMP is the recommended type for new time columns; LONG DATETIME FORMAT '1:MILLISECONDS:EPOCH' is still accepted for tables migrating from existing JSON configs. For a TIMESTAMP-typed column, Pinot's DateTimeFieldSpec treats the format as implicit and stores it as the short token 'TIMESTAMP' regardless of what the user supplied — the canonical DDL emitted by SHOW CREATE TABLE reflects that.

Create a REALTIME table with Kafka stream config

CREATE TABLE clicks (
  user_id STRING DIMENSION,
  url     STRING DIMENSION,
  ts      TIMESTAMP DATETIME FORMAT 'TIMESTAMP' GRANULARITY '1:MILLISECONDS'
)
TABLE_TYPE = REALTIME
PROPERTIES (
  'timeColumnName' = 'ts',
  'replication' = '2',
  'stream.kafka.topic.name' = 'click_events',
  'stream.kafka.consumer.factory.class.name' = 'org.apache.pinot.plugin.stream.kafka20.KafkaConsumerFactory',
  'stream.kafka.broker.list' = 'kafka-broker:9092',
  'realtime.segment.flush.threshold.rows' = '500000'
);

Multi-value (array) dimension

CREATE TABLE products (
  id   INT DIMENSION,
  tags STRING DIMENSION ARRAY
)
TABLE_TYPE = OFFLINE;

Idempotent re-creation with IF NOT EXISTS

CREATE TABLE IF NOT EXISTS events (id INT) TABLE_TYPE = OFFLINE;
-- Returns 200 if the table already exists; 201 on first creation.

Defaults and NOT NULL

CREATE TABLE users (
  id    INT NOT NULL DIMENSION,
  name  STRING NOT NULL DEFAULT 'unknown' DIMENSION,
  score DOUBLE DEFAULT 0.0 METRIC
)
TABLE_TYPE = OFFLINE;

Upsert table with PRIMARY KEY

CREATE TABLE upsertOrders (
  orderId INT NOT NULL DIMENSION,
  userId  STRING NOT NULL DIMENSION,
  amount  DOUBLE METRIC,
  ts      TIMESTAMP DATETIME FORMAT 'TIMESTAMP' GRANULARITY '1:MILLISECONDS'
) PRIMARY KEY (orderId)
TABLE_TYPE = REALTIME
PROPERTIES (
  'timeColumnName' = 'ts',
  'replication' = '2',
  'upsertConfig' = '{"mode":"FULL"}',
  'stream.kafka.topic.name' = 'orders'
);

DB-qualified table

CREATE TABLE analytics.events (id INT) TABLE_TYPE = OFFLINE;
SHOW CREATE TABLE analytics.events;
DROP TABLE analytics.events;

(The Database: HTTP header is also honored; the SQL-side db. qualifier wins when both are present.)

DROP TABLE

DROP TABLE events;                          -- drops both OFFLINE and REALTIME variants
DROP TABLE events TYPE OFFLINE;             -- drops a single variant
DROP TABLE IF EXISTS analytics.events;      -- 200 OK whether the table exists or not

SHOW

SHOW TABLES;                       -- all tables in the database from the Database header
SHOW TABLES FROM analytics;        -- override the database
SHOW CREATE TABLE events;          -- canonical DDL for the table
SHOW CREATE TABLE events TYPE REALTIME;

REST endpoint contract

All DDL operations are sent to a single endpoint:

POST /sql/ddl[?dryRun=true|false]
Content-Type: application/json
Authorization: <Bearer | Basic …>
Database: <optional database name>

{ "sql": "<single DDL statement>" }

Response codes

Code Operation Meaning
200 OK DROP / SHOW / dry-run / IF [NOT] EXISTS no-op Operation succeeded; body has the operation outcome.
201 Created CREATE Table was created and persisted to ZK.
400 Bad Request All Parse error, semantic error, oversize input (>256K chars), unsupported emitter type, or type-incompatible default value.
404 Not Found DROP / SHOW CREATE Table or schema not found (DROP without IF EXISTS, or SHOW CREATE on a missing table).
409 Conflict CREATE / DROP Duplicate CREATE without IF NOT EXISTS; logical-table reference blocking DROP; race lost to a concurrent writer.
500 Internal Server Error All Helix/ZK inconsistency, or controller defect.

Curl example

curl -X POST 'http://controller:9000/sql/ddl' \
  -H 'Content-Type: application/json' \
  -H 'Database: analytics' \
  -d '{"sql": "CREATE TABLE events (id INT) TABLE_TYPE = OFFLINE"}'

Dry-run (compile and validate without persisting)

curl -X POST 'http://controller:9000/sql/ddl?dryRun=true' \
  -H 'Content-Type: application/json' \
  -d '{"sql": "CREATE TABLE events (id INT) TABLE_TYPE = OFFLINE"}'

A dry-run produces the full compiled Schema + TableConfig payload and runs the entire validation pipeline; it persists nothing. Use it to validate DDL before applying.

Response shape (CREATE)

{
  "operation": "CREATE_TABLE",
  "tableName": "analytics.events_OFFLINE",
  "tableType": "OFFLINE",
  "databaseName": "analytics",
  "dryRun": false,
  "ifNotExists": false,
  "schema":      { /* fully-resolved Schema JSON */ },
  "tableConfig": { /* fully-resolved post-tuner TableConfig JSON */ },
  "warnings":    [ /* non-fatal compile-time warnings */ ],
  "message":     "Successfully created table analytics.events_OFFLINE"
}

Field absence semantics: every field uses @JsonInclude(NON_NULL). dryRun is emitted on CREATE and DROP responses (both have working dry-run paths) and elided on SHOW responses where the concept does not apply. ifNotExists, ifExists, tableType, warnings, deletedTables, tableNames, ddl, databaseName are absent when not relevant for the operation.


Property routing reference

The DDL PROPERTIES (...) clause is the escape hatch for everything not expressed by first-class column attributes (NOT NULL, DEFAULT, role suffix). Routing rules — applied in order:

Rule Pattern Routes to Examples
1. Promoted scalar known builder field TableConfigBuilder.setX(…) replication, brokerTenant, serverTenant, timeColumnName, retentionTimeUnit, retentionTimeValue, loadMode, sortedColumn, nullHandlingEnabled, aggregateMetrics, peerSegmentDownloadScheme, crypterClassName, segmentVersion, description, tags, isDimTable, replicasPerPartition, invertedIndexColumns, noDictionaryColumns, bloomFilterColumns, rangeIndexColumns, jsonIndexColumns, varLengthDictionaryColumns, onHeapDictionaryColumns, deletedSegmentsRetentionPeriod
2. JSON-blob known nested config TableConfigBuilder.setX(JSON.parse(value)) ingestionConfig, upsertConfig, dedupConfig, dimensionTableConfig, routingConfig, queryConfig, quotaConfig, tierConfigs, tunerConfigs, fieldConfigs, instanceAssignmentConfigMap, tagOverrideConfig, replicaGroupStrategyConfig, completionConfig, starTreeIndexConfigs, segmentPartitionConfig, multiColumnTextIndexConfig, jsonIndexConfigs, instancePartitionsMap, segmentAssignmentConfigMap, tableSamplers, tierOverwrites
3. Stream prefix stream.*, realtime.* IndexingConfig.streamConfigs (verbatim, prefix preserved). REALTIME-only. stream.kafka.topic.name, stream.kafka.consumer.factory.class.name, realtime.segment.flush.threshold.rows
4. Task prefix task.<taskType>.<key> TableTaskConfig.taskTypeConfigsMap[taskType][key] task.RealtimeToOfflineSegmentsTask.bucketTimePeriod
5. Custom (fallback) anything else TableCustomConfig.customConfigs ourTeam.flag, internal.config.X

CSV list values (rule 1 with list-typed builder fields like invertedIndexColumns) are split on commas. The free-form pass-through (rules 2-5) is the forward-compat hook: stream, minion-task, and nested-config schemas can evolve without grammar changes.

JSON-blob example: ingestion config

CREATE TABLE events (
  id INT DIMENSION,
  ts TIMESTAMP DATETIME FORMAT 'TIMESTAMP' GRANULARITY '1:MILLISECONDS'
)
TABLE_TYPE = OFFLINE
PROPERTIES (
  'timeColumnName' = 'ts',
  'ingestionConfig' = '{"batchIngestionConfig":{"segmentIngestionType":"APPEND","segmentIngestionFrequency":"DAILY"}}'
);

Task config example

CREATE TABLE events (id INT) TABLE_TYPE = OFFLINE
PROPERTIES (
  'task.RealtimeToOfflineSegmentsTask.bucketTimePeriod' = '1d',
  'task.RealtimeToOfflineSegmentsTask.maxNumRecordsPerSegment' = '5000000',
  'task.SegmentRefreshTask.tableMaxNumTasks' = '5'
);

Data types

DDL alias Pinot type Notes
INT, INTEGER INT
BIGINT, LONG LONG
FLOAT, REAL FLOAT
DOUBLE DOUBLE
DECIMAL, NUMERIC, BIG_DECIMAL BIG_DECIMAL DECIMAL(p,s) accepted but precision/scale are not enforced — a compile-time warning is emitted.
BOOLEAN BOOLEAN Stored as INT 0/1 internally; canonical DDL emits TRUE/FALSE.
TIMESTAMP TIMESTAMP Recommended for time columns. Format is implicit ('TIMESTAMP'); defaults emit as quoted UTC ISO-8601 ('2023-11-14T22:13:20Z').
VARCHAR, CHAR, STRING STRING
VARBINARY, BINARY, BYTES BYTES Defaults emit as quoted hex ('deadbeef').
JSON JSON
SMALLINT, TINYINT (rejected) Intentionally rejected to keep room for future narrow integer types.

Column roles: DIMENSION (default), DIMENSION ARRAY (multi-value), METRIC (numeric only), DATETIME FORMAT '<fmt>' GRANULARITY '<gran>'.


Authorization

  • DDL operations are authorized after database-name translation, against the post-translation db.tableName_TYPE resource. This prevents a Database: header from substituting the resource that auth was checked against (cross-DB privilege escalation).
  • DROP TABLE and SHOW CREATE TABLE without a TYPE clause require permission on both OFFLINE and REALTIME variants — the no-fingerprinting contract under per-type ACL plugins. Partial-permission callers must use TYPE OFFLINE | REALTIME to read or drop a single variant.

Forward and reverse round-trip

SHOW CREATE TABLE produces a deterministic canonical DDL string with:

  • Properties emitted in lexicographic order.
  • BIG_DECIMAL defaults via toPlainString() (no scientific notation).
  • BOOLEAN defaults as TRUE/FALSE.
  • TIMESTAMP defaults as UTC ISO-8601.
  • BYTES defaults as quoted hex.
  • Identifiers that collide with reserved/data-type/DDL keywords double-quoted (e.g. a column named int or metric is emitted as "int" / "metric").
  • Custom-config keys that would shadow promoted scalars or JSON-blob keys are rejected at emit time to prevent silent round-trip corruption.

emit → parse → compile → emit is idempotent: the RoundTripTest suite exercises every routing rule against this property, including the TIMESTAMP-typed time column and the BOOLEAN DEFAULT TRUE round-trip (where Pinot's natural-default elision intentionally omits DEFAULT FALSE).


Rolling-upgrade safety

The DDL feature is purely additive. No SPI signature, no enum, no TableConfig/Schema field, no wire format, and no ZK property-store path is renamed or removed. The persisted artifacts produced by POST /sql/ddl are shape-identical to those produced by the existing POST /tables and POST /schemas endpoints — old brokers, servers, and controllers read DDL-created tables exactly as they read JSON-API-created tables.

Concretely:

  • Pre-DDL controllers return 404 on POST /sql/ddl. There is no half-state — the endpoint either exists or it does not.
  • New SQL keywords (DIMENSION, METRIC, GRANULARITY, OFFLINE, REALTIME, PROPERTIES, TABLES, TABLE_TYPE, IF) are added under nonReservedKeywordsToAdd in config.fmpp. Existing DQL queries that use dimension, metric, format, etc. as identifiers continue to parse on the new binary. Pre-DDL binaries never saw these tokens at all.
  • Rollout direction is irrelevant. Upgrading controllers first, brokers first, servers first, or any interleaving all produce the same on-disk representation. Rollback to a pre-DDL controller continues to serve DDL-created tables identically to JSON-API tables.

See pinot-sql-ddl/DESIGN.md §8.1 for the full mixed-version posture.


Test plan

  • 31 parser tests (PinotDdlParserTest) — all syntax variants, IF NOT EXISTS, IF EXISTS, SHOW CREATE TABLE, SHOW TABLES FROM <db>, keyword-as-identifier, PRIMARY KEY parens regression, malformed-PK error message
  • 41 compiler tests (DdlCompilerTest) — every property routing rule, all data type aliases, role inference, type compatibility for DEFAULT, negative cases (DEFAULT NULL, type mismatch, SMALLINT/TINYINT rejection, lowercase TABLE_TYPE, missing PK column), DECIMAL precision warning, stream/realtime/task routing, JSON-blob round-trip
  • 21 emitter golden-output tests (CanonicalDdlEmitterTest) — canonical clause order, lexicographic property ordering, BIG_DECIMAL plain-string, BIG_DECIMAL scale-shifted natural-default elision, BOOLEAN TRUE/FALSE, TIMESTAMP UTC ISO-8601, BYTES non-natural-default hex, ComplexFieldSpec/MAP/LIST/STRUCT explicit rejection, custom-config key shadowing rejection
  • 24 round-trip tests (RoundTripTest) — emit → parse → compile → emit idempotence across stream configs, task configs, custom configs, ingestion JSON-blob, MV dimensions, primary keys, identifiers needing quoting, kitchen-sink promoted-scalars regression, TIMESTAMP time column, BOOLEAN DEFAULT TRUE (with natural-default elision for FALSE)
  • 18 unit tests (PinotDdlRestletResourceUnitTest) — describeColumnShapeMismatch covers data type, field type, single-value, NOT NULL, default-null-value, BYTES content equality, BIG_DECIMAL compareTo equivalence, DATETIME format/granularity
  • 17 controller integration tests (PinotDdlRestletResourceTest) — CREATE/DROP/SHOW round-trip on a real ControllerTest cluster, dry-run, IF [NOT] EXISTS, 201/200/404/409/400 codes, DB-qualified DDL, oversize input rejection, parse-error and semantic-error 400s

All tests pass. ./mvnw spotless:apply checkstyle:check license:format license:check clean across pinot-common, pinot-sql-ddl, and pinot-controller.


Review history

This branch went through multiple review iterations driven by the code-review-orchestrator and code-reviewer agents across the eight defect domains (config-backcompat, concurrency-state, architecture, performance, correctness-nulls, testing, naming-api, process-scope). Headline issues addressed by regression tests: parser PRIMARY KEY LOOKAHEAD, BYTES typed comparison + emit, BIG_DECIMAL scale-sensitive equality, ComplexFieldSpec silent drop, validation divergence vs POST /tables, hybrid second-variant CREATE schema metadata, TIMESTAMP UTC emission, SHOW CREATE auth fingerprinting, DEFAULT NULL rejection, type-compatible default validation, BOOLEAN default round-trip, and the dryRun wire-shape asymmetry on SHOW responses.

🤖 Generated with Claude Code

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 67.55181% with 501 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.72%. Comparing base (11545db) to head (11dd972).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
...troller/api/resources/PinotDdlRestletResource.java 57.00% 143 Missing and 41 partials ⚠️
.../apache/pinot/sql/ddl/compile/PropertyMapping.java 65.82% 72 Missing and 9 partials ⚠️
...pache/pinot/sql/ddl/reverse/PropertyExtractor.java 65.19% 33 Missing and 38 partials ⚠️
.../pinot/sql/parsers/parser/SqlPinotCreateTable.java 31.91% 32 Missing ⚠️
.../org/apache/pinot/sql/ddl/compile/DdlCompiler.java 84.50% 17 Missing and 14 partials ⚠️
...rg/apache/pinot/sql/ddl/reverse/SchemaEmitter.java 70.32% 14 Missing and 13 partials ⚠️
.../sql/parsers/parser/SqlPinotColumnDeclaration.java 46.34% 22 Missing ⚠️
...che/pinot/sql/ddl/reverse/CanonicalDdlEmitter.java 80.95% 6 Missing and 6 partials ⚠️
...he/pinot/sql/parsers/parser/SqlPinotDropTable.java 47.36% 10 Missing ⚠️
...che/pinot/sql/parsers/parser/SqlPinotProperty.java 46.66% 8 Missing ⚠️
... and 7 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18241      +/-   ##
============================================
+ Coverage     63.70%   63.72%   +0.02%     
- Complexity     1684     1932     +248     
============================================
  Files          3266     3292      +26     
  Lines        199898   201470    +1572     
  Branches      31048    31316     +268     
============================================
+ Hits         127338   128388    +1050     
- Misses        62409    62794     +385     
- Partials      10151    10288     +137     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.72% <67.55%> (+0.02%) ⬆️
temurin 63.72% <67.55%> (+0.02%) ⬆️
unittests 63.72% <67.55%> (+0.02%) ⬆️
unittests1 55.78% <40.90%> (-0.03%) ⬇️
unittests2 35.24% <67.55%> (+0.27%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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 feature/sql-ddl-slices-1-2 branch 4 times, most recently from f983625 to f56db2e Compare April 23, 2026 10:44
@xiangfu0 xiangfu0 force-pushed the feature/sql-ddl-slices-1-2 branch from 888ead3 to 80a1d5d Compare April 27, 2026 01:54
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request Apr 27, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xiangfu0 xiangfu0 force-pushed the feature/sql-ddl-slices-1-2 branch from 0ad152c to 9909e62 Compare May 1, 2026 07:01
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 1, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 4, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 8, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 8, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 8, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xiangfu0 xiangfu0 force-pushed the feature/sql-ddl-slices-1-2 branch from 9909e62 to fb33fc0 Compare May 9, 2026 06:44
xiangfu0 added a commit to xiangfu0/pinot that referenced this pull request May 11, 2026
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@xiangfu0 xiangfu0 force-pushed the feature/sql-ddl-slices-1-2 branch from fb33fc0 to 1aef0a7 Compare May 11, 2026 11:06
@xiangfu0 xiangfu0 added the sql-compliance Related to SQL standard compliance label May 14, 2026
xiangfu0 and others added 12 commits May 14, 2026 21:55
…es 1+2)

Introduces a new `pinot-sql-ddl` module and a `/sql/ddl` controller endpoint
that compile and execute Pinot DDL statements:

- CREATE TABLE: compiles DDL → Schema + TableConfig, runs the full validation
  stack (TableConfigUtils, TaskConfigUtils, TableConfigValidatorRegistry), and
  persists via PinotHelixResourceManager. Rejects sibling-table creation when
  the DDL column list conflicts with an existing stored schema.
- DROP TABLE: drops OFFLINE, REALTIME, or both, with IF EXISTS support.
- SHOW TABLES: lists all raw table names in the resolved database scope.
- SHOW CREATE TABLE: emits canonical round-trip DDL via CanonicalDdlEmitter.

Key components:
- DdlCompiler: Calcite/JavaCC parser → ResolvedTableDefinition → Schema +
  TableConfig via PropertyMapping (promoted scalars, stream.*, task.*, JSON
  blobs, custom-config fall-through).
- PropertyExtractor: inverse of PropertyMapping; emits a deterministic
  lexicographically-sorted property map from a TableConfig for canonical DDL.
- CanonicalDdlEmitter: renders canonical CREATE TABLE SQL; preserves
  db-qualified table names and emits all sortedColumn values as CSV.
- SchemaEmitter: emits column declarations including DIMENSION ARRAY (MV)
  syntax for multi-value fields.
- Auth-before-existence checks throughout to prevent metadata leakage.
- 53 unit tests covering compiler, round-trip, and emitter paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CREATE concurrent race: skip schema rollback when TableAlreadyExistsException
  is caught but the table now exists (another caller won the race); previously
  this unconditionally deleted the shared schema leaving the winner's table
  without its schema.

- DROP TABLE: add logical-table reference check (matching /tables DELETE),
  run tableTasksCleanup before each variant deletion to remove task schedules,
  and delete the shared schema after the last physical variant is dropped so
  stale schema metadata does not block future CREATE TABLE for the same name.

- SHOW TABLES: use TargetType.CLUSTER + Actions.Cluster.GET_TABLE instead of
  the table-scoped ResourceUtils.checkPermissionAndAccess, matching the auth
  model of the existing GET /tables endpoint and avoiding 403 for callers who
  have cluster-level listing access but not a per-table READ on a resource
  named after the database.

- sortedColumn multi-column restore: PropertyMapping.apply() now returns the
  full sorted-column list; DdlCompiler applies it via IndexingConfig.setSortedColumn(List)
  after builder.build() so multi-column sort configs survive a SHOW CREATE / replay
  round-trip instead of being truncated to the first column.

- SHOW CREATE TABLE correctness:
  - Emit PRIMARY KEY (...) clause when schema.primaryKeyColumns is set so
    upsert/dedup/dimension tables round-trip without losing key metadata.
  - Emit legacy timeFieldSpec columns as DATETIME so they are not silently
    dropped on replay.
  - Fail fast with IllegalArgumentException for MAP/LIST/STRUCT columns whose
    types have no DDL representation, rather than emitting lossy DDL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- PRIMARY KEY: add grammar production, AST field, compiler extraction,
  and emitter (outside column list, before TABLE_TYPE) so SHOW CREATE
  TABLE output re-parses correctly
- sortedColumn multi-column: PropertyMapping.apply() now returns the
  full sorted-column list; DdlCompiler applies it via
  IndexingConfig.setSortedColumn(List) after builder.build()
- TimeFieldSpec granularity: SchemaEmitter.emitTimeColumn() uses
  tgs.getTimeUnitSize() instead of hardcoded '1' so a 15-MINUTE spec
  emits '15:MINUTES' not '1:MINUTES'
- Add PRIMARY KEY parser tests (3), compiler tests (3), and round-trip
  tests (3) covering single-key, composite-key, absent-key, granularity
  correctness, and datetime format preservation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SqlPinotCreateTable.getOperandList() now includes _primaryKeyColumns
  so Calcite AST traversal (SqlShuttle, validators, node equality) sees
  the full operand set
- DdlCompiler validates PRIMARY KEY columns exist in the column list;
  referencing an undeclared name now throws DdlCompilationException
- PinotDdlRestletResource: run validateTableConfig() BEFORE the
  IF NOT EXISTS existence check so invalid DDL is rejected even when
  the table already exists (IF NOT EXISTS is a no-op only for valid
  statements)
- Tests: add primaryKeyReferencingUnknownColumnThrows compiler test;
  replace assertEquals(bool, true/false) with assertTrue/assertFalse

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- replicasPerPartition: add extractor emission, PropertyMapping
  consumed-key registration, and case-insensitive post-build apply in
  DdlCompiler so SHOW CREATE TABLE preserves per-partition replica count
- SHOW CREATE TABLE: catch IllegalArgumentException from
  CanonicalDdlEmitter (unsupported column types MAP/LIST/STRUCT) and
  return 400 instead of 500
- DROP TABLE: per-target try/catch loop reports partial outcomes instead
  of leaving hybrid tables half-deleted; null-safe getMessage() in
  partial-failure error message
- CREATE rollback: generic catch block re-checks both OFFLINE and
  REALTIME variant existence before deleting shared schema so concurrent
  sibling-variant creates are not orphaned
- Dry-run existence check: apply hasTable check for both dryRun=true and
  live paths so dry-run faithfully predicts 409 conflicts
- SqlPinotCreateTable.getOperandList(): null-guard for absent PRIMARY KEY
  so Calcite framework iterators do not NPE on tables without a PK clause

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…SON comparison, SHOW CREATE TABLE hybrid ambiguity, dry-run existence check, DECIMAL precision warning, HTTP 201 for CREATE TABLE

- SqlPinotShowTables: add @nullable on _database field declaration
- PinotDdlRestletResource: guard against >256 KB SQL inputs; use JsonNode.equals()
  for schema comparison to avoid ordering-dependent false mismatches; reject SHOW
  CREATE TABLE without TYPE when both OFFLINE and REALTIME variants exist; drop the
  !dryRun guard so dry-run faithfully predicts conflicts; return HTTP 201 for
  successful non-dry-run CREATE TABLE, 200 for everything else
- DdlCompiler: emit advisory warning when DECIMAL/NUMERIC precision+scale are
  specified (Pinot BIG_DECIMAL does not enforce them); pass warnings list into
  resolveColumns() so the warning can be recorded during column resolution

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ne FQCN, variable-arity getOperandList, and missing tests

- DdlCompiler: gate DECIMAL/NUMERIC precision warning on whether the user actually
  specified DECIMAL(p,s) by checking SqlBasicTypeNameSpec.getPrecision() against
  RelDataType.PRECISION_NOT_SPECIFIED (-1); bare DECIMAL no longer emits a spurious
  warning
- PinotDdlRestletResource: replace fully-qualified com.fasterxml.jackson.databind.JsonNode
  inline references with the already-imported short form
- SqlPinotCreateTable.getOperandList(): use fixed-arity list with null placeholder for
  the optional _primaryKeyColumns instead of a variable-length list that shifts all
  subsequent positional indices when PRIMARY KEY is absent
- DdlCompilerTest: add decimalWithPrecisionScaleEmitsWarning and
  decimalWithoutPrecisionEmitsNoWarning regression tests
- PinotDdlRestletResourceTest: add successfulCreateReturns201 and
  oversizedInputReturnsBadRequest integration tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…aType in DdlCompiler

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…anup, test helper

- Replace full-JSON hybrid-table schema comparison with a column-shape
  comparator that covers only attributes the DDL column list actually
  expresses (name, dataType, fieldType, singleValue, NOT NULL, default,
  DATETIME format/granularity) and ignores schema-level metadata the DDL
  cannot set (primaryKeyColumns, tags, enableColumnBasedNullHandling).
  Prevents false CONFLICT when the first hybrid variant set primary keys
  and the second variant's DDL does not restate them.
- Remove auto-schema-deletion from DROP TABLE so the DDL path matches
  the existing /tables/{name} DELETE contract, which leaves the shared
  schema intact when the last variant is removed.
- Replace fragile substring-scan for HTTP status codes in
  postRawExpectFailureBody with postRequestWithStatusCode, which reads
  the status from the response directly. Also fix a stale test
  assertion that expected an unquoted "default" in canonical DDL (the
  emitter correctly quotes it because DEFAULT is a reserved keyword).
- Add PinotDdlRestletResourceUnitTest covering acceptance when PK
  metadata differs and rejection for data-type, field-type,
  single-value, NOT NULL, default-null-value, and DATETIME format /
  granularity mismatches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… pass

CRITICAL:
- CREATE TABLE IF NOT EXISTS now matches standard SQL semantics: the
  existence check runs before validateTableConfig, so an idempotent
  re-CREATE against an existing table succeeds even if a config validator
  rule has tightened between Pinot versions.
- DROP TABLE no longer collapses ControllerApplicationException from
  tableTasksCleanup (e.g. BAD_REQUEST for active running tasks) into
  500. The first-failure status code is preserved end-to-end and the
  partial-failure error message lists both successful and failed
  variants with a recovery hint.
- PRIMARY KEY parser uses LOOKAHEAD(3) <PRIMARY> <KEY> <LPAREN> instead
  of LOOKAHEAD(2), so a malformed PRIMARY KEY id (no parens) backtracks
  to the expected-TABLE_TYPE error path instead of producing a confusing
  inner-grammar error. Added a parser test covering the failure mode.
- DROP TABLE partial-failure path is now explicitly tested via the
  improved error message and status preservation.

MAJOR:
- CREATE TABLE schema rollback no longer races. Previously the catch
  branch read hasOfflineTable + hasRealtimeTable non-atomically before
  deleting the schema; a concurrent sibling CREATE could win between
  the two reads and have its schema deleted out from under it. Pinot
  already supports schema-outlives-table; we surface a hint pointing
  the operator at DELETE /schemas/{name} for permanent failures.
- validateTableConfig now distinguishes user errors
  (IllegalArgument/IllegalStateException → 400) from controller defects
  (any other exception → 500), so monitoring picks up internal failures
  instead of mislabeling them as malformed DDL.
- SHOW CREATE TABLE narrows the IllegalArgumentException catch to
  documented emission failures (unsupported column types, reserved-key
  collisions) and treats other RuntimeExceptions as 500 internal
  errors.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…arser test assertion

- Downgrade the per-target log inside the typed ControllerApplicationException
  catch in executeDrop from ERROR with throwable to WARN with just the
  message. The CAE constructor already logged the underlying error, and
  the wrapping CAE thrown after the loop logs again — three entries for
  one user-actionable 4xx is noise (C7.10).
- Strengthen createTableMissingPrimaryKeyParensRejected to assert the
  parser error message names TABLE_TYPE, locking in the expected error
  fragment so a regression in the LOOKAHEAD widening cannot pass the
  test silently (C6.5).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CRITICAL:
- SHOW CREATE TABLE no-TYPE branch now authorizes only the variant(s)
  it would actually surface, not both unconditionally. The previous
  "authorize both up-front" pattern caused a regression for callers
  with READ on only one variant: they would get 403 even when only
  the authorized variant existed and the read should succeed. The
  no-fingerprinting goal (don't let unauthorized callers distinguish
  exists-but-forbidden from not-found) is preserved by running an
  auth check against the OFFLINE candidate before surfacing 404.

MAJOR:
- CREATE TABLE for the second hybrid variant now validates against
  the stored schema (which carries the canonical primary keys, tags,
  null-handling) rather than the DDL-compiled schema. Previously an
  upsert table whose first variant declared PRIMARY KEY would fail
  validation when the second variant's DDL omitted the PK clause,
  even though describeColumnShapeMismatch correctly accepted the
  metadata difference.
- SHOW CREATE TABLE: when hasTable=true but getTableConfig returns
  null, surface 500 with a "torn write or concurrent delete" message
  instead of masking ZK inconsistency as 404.
- DdlCompiler.extractLiteralValue now throws DdlCompilationException
  for non-SqlLiteral inputs instead of silently calling toString(),
  preventing a future grammar relaxation from leaking SQL-wire
  quoting into FieldSpec.defaultNullValue.
- PropertyMapping.applyPromoted now eagerly validates
  replicasPerPartition as an integer, mirroring the replication path.
- DROP TABLE unexpected-Exception branch logs a one-line breadcrumb
  at WARN instead of a full stack trace; the wrapping CAE thrown
  after the loop already logs the firstFailure with full context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xiangfu0 and others added 16 commits May 14, 2026 21:55
…t double-auth

The previous commit's per-variant auth-after-existence pattern preserved the
"legitimate one-variant reader" case but introduced a subtle fingerprinting
leak under access-control plugins that grant per-type permissions (the SPI
permits this and enterprise plugins use it). A caller with READ on OFFLINE
only could distinguish "REALTIME-only-exists" (403) from "neither-exists"
(404), revealing the existence of a REALTIME variant they shouldn't see.

Restore the up-front double-authorize pattern (matching the DROP TABLE
symmetry) and direct partial-permission callers to use the explicit
TYPE clause to read a single variant. The bare SHOW CREATE TABLE form
is intentionally more restrictive because answering it requires reading
both candidates' state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…view pass

CRITICAL:
- PRIMARY KEY parser was double-consuming <PRIMARY> <KEY> <LPAREN>: my
  earlier LOOKAHEAD widening had explicit token consumption at the call
  site that conflicted with the rule body, breaking every CREATE TABLE
  ... PRIMARY KEY (...) statement. The advertised PRIMARY KEY support
  has been dead since LOOKAHEAD(3) was introduced.
  Fix: drop the inline tokens at the call site; the LOOKAHEAD(3)
  integer alone correctly peeks at the next 3 tokens of the production
  (which are <PRIMARY> <KEY> <LPAREN>) without consuming them. The
  previously-failing createTableWithPrimaryKey,
  createTableWithCompositePrimaryKey, and
  createTableMissingPrimaryKeyParensRejected tests now pass.

MAJOR:
- describeColumnShapeMismatch now compares typed default values via
  Objects.equals(getDefaultNullValue(), ...) rather than
  getDefaultNullValueString(). The string form produces false 0 vs
  0.0 mismatches when the same numeric default arrived via different
  literal forms (DDL "DEFAULT 0.0" vs JSON-API integer 0); the typed
  projection collapses them through FieldSpec's data-type parsing.
- Added a clarifying comment at the deleteTable call site documenting
  that the helper takes the raw name + explicit TableType and derives
  the typed name internally via TableNameBuilder, addressing reviewer
  uncertainty about the calling convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit's switch to typed Objects.equals on getDefaultNullValue()
broke BYTES columns: getDefaultNullValue() allocates a fresh byte[] on each
call (FieldSpec dataType.convert), so Objects.equals does reference
comparison, not content comparison, falsely flagging every hybrid
second-variant CREATE on a BYTES column with a custom default as a
mismatch.

Fix: use FieldSpec.DataType.equals(v1, v2) which delegates to Arrays.equals
for BYTES and to value.equals for other types. Added two regression tests:
matching BYTES defaults must accept; differing BYTES defaults must reject.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… review pass

CRITICAL:
- SchemaEmitter.emitColumn used Object.equals to compare default value
  against the natural default. For BYTES columns, both sides are byte[]
  whose .equals() is reference equality, so every BYTES column at
  natural default was emitting a redundant DEFAULT '<hex>' clause —
  same bug pattern that was just fixed in describeColumnShapeMismatch.
  Fix: use FieldSpec.DataType.equals(v1, v2), which delegates to
  Arrays.equals for BYTES.

MAJOR:
- BIG_DECIMAL DEFAULT now emits BigDecimal.toPlainString() instead of
  toString(); the latter can emit scientific notation (1E+10) which
  Calcite's Literal() rule does not accept, breaking round-trip for
  large or small magnitudes.
- DROP TABLE failure aggregation now tracks the integer status code
  rather than the Response.Status enum. fromStatusCode returns null
  for non-standard codes (422, 423, 451), which would silently fall
  back to 500 and hide the original 4xx information.
- DEFAULT NULL is now rejected explicitly with DdlCompilationException
  instead of silently behaving as if no DEFAULT was supplied.
  SqlLiteral.toValue returns null for SqlLiteral.createNull, and the
  previous code path stored that as the column's defaultValue then
  guarded against it later — a meaningless DDL became a silent no-op.
- Removed the schema-cleanup hint from the 500-error message in the
  CREATE TABLE generic-failure path. The hint pointed callers at
  DELETE /schemas/{name}, encouraging premature cleanup in response
  to transient ZK/Helix failures. The schema persists either way (per
  the existing /tables contract) and operators can investigate via
  logs and metrics.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per C6.3 (bug fixes require regression tests), add three focused tests
covering the fixes in the prior commit:

- noDefaultEmittedForBytesAtNaturalDefault: a BYTES dimension at its
  natural default must not emit a DEFAULT clause. Without the
  DataType.equals fix, Object.equals on byte[] would fail content
  comparison and the canonical output would carry a redundant
  DEFAULT '<hex>' clause.
- bigDecimalDefaultEmitsPlainString: a BIG_DECIMAL default of 1E+30
  must round-trip without scientific notation. Calcite's Literal()
  rule does not accept scientific notation, so without
  toPlainString() the round-trip would fail to re-parse.
- defaultNullRejectedExplicitly: DEFAULT NULL must throw
  DdlCompilationException with a message naming DEFAULT NULL, instead
  of silently behaving as if no DEFAULT was supplied.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…view pass

CRITICAL:
- DDL CREATE was running a subset of the validation pipeline that
  POST /tables uses. Operators relying on min-replicas enforcement,
  storage-quota constraints, hybrid-pair compatibility, or instance-
  assignment validation could see DDL CREATE silently bypass them.
  Fix: delegate to TableConfigValidationUtils.validateTableConfig,
  the same canonical helper /tables uses. Inject ControllerConf and
  remove the bespoke validateTableConfig body.

MAJOR:
- Apply TableConfigTunerUtils.applyTunerConfigs before validation,
  mirroring POST /tables. Without this, a tuner-introduced config
  (e.g. a defaulted index config) could bypass validation.
- SchemaEmitter.emitColumns now skips the legacy TimeFieldSpec when
  a DateTimeFieldSpec with the same column name already exists.
  Schemas mid-migration carry both, and emitting both yielded a
  duplicate-column declaration that fails to re-parse.
- Reduced PRIMARY KEY LOOKAHEAD(3) to LOOKAHEAD(2). Two-token
  disambiguation (<PRIMARY> <KEY>) is sufficient and produces a
  more accurate error message ("expected (") for malformed
  PRIMARY KEY id (no parens) cases. Updated the regression test to
  reflect the post-commit-error path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit ran TableConfigTunerUtils.applyTunerConfigs AFTER
the DdlExecutionResponse was built, so the response advertised the
pre-tuner config while ZK persists (and dry-run would mis-predict)
the post-tuner shape. Refresh response.setTableConfig() right after
applyTunerConfigs runs so dry-run and live-create both surface the
shape that will actually be persisted.

Also strengthen the createTableMissingPrimaryKeyParensRejected test
to assert the error message names the missing LPAREN, locking in the
LOOKAHEAD(2) commit-then-fail-at-LPAREN behavior described in the
test comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…review pass

CRITICAL:
- SchemaEmitter.emitColumns iterated only Dimension/Metric/DateTime/Time
  field specs, silently dropping ComplexFieldSpec columns from canonical
  DDL output. validateEmittable was never called for them. Replay of the
  emitted DDL would have produced a schema missing those columns —
  exactly the silent-drop scenario the validation was added to prevent.
  Fix: explicit fail-fast iteration over getComplexFieldSpecs() with a
  clear error pointing at the unsupported COMPLEX field type.

MAJOR:
- BIG_DECIMAL default-null-value comparison now uses compareTo == 0
  instead of equals. BigDecimal.equals is scale-sensitive
  (new BigDecimal("1").equals(new BigDecimal("1.0")) is false), which
  produced phantom mismatches when the same numeric default arrived via
  different literal forms. Applied in both
  describeColumnShapeMismatch (controller) and SchemaEmitter (natural
  default check).
- emitDefault now emits SQL TRUE/FALSE for BOOLEAN columns and quoted
  ISO timestamp strings for TIMESTAMP columns. Pinot stores these as
  Integer 0/1 and Long millis respectively; without this, canonical
  DDL exposed the internal encoding instead of the SQL literal form.
- DataTypeMapper now rejects SMALLINT/TINYINT explicitly with a clear
  error pointing at INT. Silently widening to INT today would lock
  every existing SMALLINT/TINYINT DDL into INT semantics permanently
  if Pinot ever adds INT8/INT16; rejected types can later become
  accepted, silently-promoted ones cannot be narrowed without breaking
  users.
- CompiledShowCreateTable.getTableType() Javadoc corrected: said
  "defaults to OFFLINE when both variants exist" but the actual
  behavior is to return 400 BAD_REQUEST on hybrid pairs and require
  the caller to specify TYPE OFFLINE | REALTIME. Future implementers
  reading the SPI doc would have produced the wrong policy.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MAJOR:
- TIMESTAMP DEFAULT emit now uses Instant.ofEpochMilli().toString()
  (UTC ISO-8601) instead of new Timestamp(...).toString() which
  formats in the JVM's default time zone. The previous form would
  emit different canonical DDL strings for the same input on
  controllers in different time zones, defeating the
  canonical-DDL-output contract.

Regression tests (per C6.3):
- bigDecimalAtNaturalDefaultDoesNotEmitDefault: locks in compareTo
  equality so BigDecimal("0.0") matches the natural BigDecimal.ZERO
  and no redundant DEFAULT clause is emitted.
- booleanDefaultEmittedAsSqlLiteral: locks in TRUE/FALSE emission
  rather than the internal Integer 0/1 form.
- timestampDefaultEmittedInUtcIso: locks in UTC ISO-8601 emission
  with Instant.toString output (validated against epoch
  1700000000000 → "2023-11-14T22:13:20Z").
- smallintTinyintRejectedExplicitly (DdlCompilerTest): locks in the
  explicit rejection of SMALLINT/TINYINT with messages naming the
  type, preventing future silent INT promotion.
- acceptsScaleShiftedBigDecimalDefault (PinotDdlRestletResourceUnitTest):
  locks in compareTo equality in the controller-side comparator so
  hybrid CREATE accepts BigDecimal("1") vs BigDecimal("1.0") as
  equivalent.

ComplexFieldSpec rejection cannot be unit-tested directly (the
constructor itself rejects DataType.STRUCT via the FieldSpec base
class), so the production guard is purely defensive against future
schema-construction paths that bypass the base-class check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- PropertyMapping class-level Javadoc said the stream. prefix is
  stripped; the implementation actually preserves it (and routes
  realtime.* keys the same way). Updated the contract description to
  match implementation: keys are stored verbatim with their
  stream./realtime. prefix intact.
- DROP TABLE: documented the no-TYPE form's "authorize both variants
  up-front" contract inline at the candidates loop, mirroring the
  SHOW CREATE policy and explaining the partial-permission caller's
  workaround (use explicit TYPE clause).
- DROP TABLE partial-failure: now tracks targets whose
  tableTasksCleanup ran but whose deleteTable subsequently failed,
  and surfaces them in the error message with a recovery hint
  pointing at the TableConfig taskTypeConfigsMap.SCHEDULE_KEY entries
  the operator must restore.
- MAX_DDL_SQL_LENGTH renamed to MAX_DDL_SQL_CHARS; Javadoc clarifies
  the limit is in Java characters (UTF-16 code units), not bytes,
  with a note for operators sizing reverse-proxy body limits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- emitTableType replaced ternary (which silently returned REALTIME for
  any non-OFFLINE TableType) with an exhaustive switch that throws
  IllegalArgumentException on unknown values. The throw maps to the
  500 path the controller resource already handles for emit() runtime
  exceptions.
- SHOW CREATE TABLE: a missing schema (after hasTable + tableConfig
  pass) now returns 404 instead of 500. Schemas can be deleted
  independently via DELETE /schemas/{name}, so a missing schema is
  reachable in normal operation and is caller-actionable. The error
  message tells the operator how to recreate it.
- DEFAULT type compatibility check: extractLiteralValue now coerces
  the default literal through DataType.convert() at compile time
  rather than letting an INT column with DEFAULT 'abc' compile cleanly
  and fail at first ingestion. Added regression test.
- Added a regression test asserting case-insensitive TABLE_TYPE input
  is accepted and canonicalized to uppercase, locking in the
  parseTableType equalsIgnoreCase contract against future grammar
  tightening.
- Added package-info.java for org.apache.pinot.sql.ddl documenting:
  module layering vs pinot-common, sub-package responsibilities,
  thread safety, exception → HTTP-status contract, and the evolution
  policy for adding new TableConfig properties / column attributes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The assertion `assertFalse(emitted.contains("DEFAULT 1") && !emitted.contains("DEFAULT TRUE"))`
was vacuous because the line above already asserted `emitted.contains("DEFAULT TRUE")` is
true, forcing the negated conjunct to false. The intent was to forbid a raw-integer
fallback, so simplify to `assertFalse(emitted.contains("DEFAULT 1"))` — which is the
actual property the test should lock in (per C6.5: tests must validate claimed behavior).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
emitDefault's switch had no BYTES case, so byte[] defaults fell through to
the default arm which calls value.toString() on a byte[] — yielding the JVM
identity-hash form like "[B@1f32e575". SHOW CREATE TABLE on a column with a
non-natural BYTES default would emit structurally invalid DDL.

Added a BYTES case that hex-encodes the byte[] via BytesUtils.toHexString,
matching the convention used by FieldSpec.getDefaultNullValueString and
toJsonObject. Added a regression test asserting the emitted DDL contains
the quoted hex string and does not leak the byte[] identity-hash form.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three test files used fully-qualified class names inline (java.math.BigDecimal,
java.util.LinkedHashMap, java.util.Map, org.apache.pinot.spi.config.table.TableTaskConfig,
TableCustomConfig) where the corresponding imports could be added. CLAUDE.md
prefers imports over FQCN; this is a style cleanup the reviewer surfaced as a
MINOR nit. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DESIGN.md covers the SQL DDL feature end-to-end: goals/non-goals, module
layering, grammar productions, type mapping, property routing rules
(promoted scalar / JSON blob / stream prefix / task prefix / custom
fallback), validation pipeline shared with POST /tables, hybrid
second-variant CREATE semantics, canonical-DDL emission contract,
REST endpoint pipeline, no-fingerprinting auth, forward-compat hooks,
backward-compat guarantees, concurrency model, test strategy across
the six test suites, known limitations, and the decision log capturing
why we chose single dispatch endpoint, TABLE_TYPE clause shape,
no-rollback CREATE, up-front double-auth, SMALLINT/TINYINT rejection,
and the separate pinot-sql-ddl module placement.

README.md is a short module entry point with a quickstart example,
the sub-package layout, and pointers to DESIGN.md, package-info.java,
and PR apache#18241 for the user-manual-style examples.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch the README quickstart and the tests that specifically validate
DateTimeFieldSpec format normalization (compiler's
columnRolesProduceCorrectFieldSpecs, emitter's allColumnRolesAndDatetime,
parser's createTableAllColumnModifiers) to use TIMESTAMP as the time-column
data type, since TIMESTAMP is the recommended shape for new tables.

Behavior surfaced: DateTimeFieldSpec silently rewrites the FORMAT string to
the bare token "TIMESTAMP" whenever the column data type is TIMESTAMP
(see pinot-spi DateTimeFieldSpec lines 78-81). The README therefore uses
the canonical short form FORMAT 'TIMESTAMP' that SHOW CREATE TABLE will
emit, so what users copy matches what the tool produces. Test comments
document the silent rewrite.

Coverage preserved on the historical LONG/EPOCH path:
- Parser negative cases (datetimeWithoutFormatFails,
  datetimeWithoutGranularityFails) and createTableRealtime still use
  LONG/EPOCH.
- Compiler property-routing tests (promotedPropertiesMapToTableConfigFields,
  streamPropertiesRoutedToStreamConfigsForRealtime) still use LONG/EPOCH.
- Emitter streamConfigsRoundTripWithOriginalKeys and the dedicated
  datetimeFieldRoundTripsFormatAndGranularity (SIMPLE_DATE_FORMAT) keep
  the LONG path.
- All 5 original RoundTripTest cases keep LONG/EPOCH.
- Controller integration tests keep LONG/EPOCH.

Added one new round-trip test (offlineTableWithTimestampTimeColumn) so the
TIMESTAMP emit -> parse -> compile -> idempotency loop is locked under
positive regression coverage.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@xiangfu0 xiangfu0 force-pushed the feature/sql-ddl-slices-1-2 branch from 1aef0a7 to 4bc6aaa Compare May 15, 2026 05:03
xiangfu0 and others added 4 commits May 14, 2026 22:23
…4 + rolling-upgrade doc + FQCN cleanup

Multi-domain review found 0 critical and 9 major findings. This commit
addresses the actionable major findings and the low-risk minor nits.

- RoundTripTest: add offlineTableWithBooleanDefault to lock in BOOLEAN
  DEFAULT TRUE end-to-end (parse -> compile -> emit -> re-parse -> emit
  idempotency) and to pin the natural-default elision behavior for
  DEFAULT FALSE. Calcite's SqlLiteral.toValue() returns "TRUE"/"FALSE";
  FieldSpec.setDefaultNullValue routes those through
  DataType.BOOLEAN.convert (BooleanUtils.toInt) to Integer 0/1. Since
  the BOOLEAN dimension natural default is Integer 0, the canonical
  emitter correctly elides DEFAULT FALSE.

- PinotDdlRestletResource @ApiResponses: add 404 (DROP without IF
  EXISTS, SHOW CREATE on missing table) and clarify 400, 409, 500
  preconditions so Swagger-generated clients know to handle the full
  failure surface.

- DESIGN.md: expand Section 8 with an explicit 8.1 "Rolling-upgrade
  safety" subsection covering mixed-version ordering, ZK schema
  immutability, pre-DDL controller behavior, rollback safety, and why
  the new SQL keywords cannot break existing DQL. Makes the
  rolling-upgrade story auditable.

- SchemaEmitter: replace inline FQCN
  org.apache.pinot.spi.data.TimeGranularitySpec with a proper import.
  Matches the project's "imports over fully qualified class names"
  convention.

- PinotDdlParserTest: replace inline java.util.Locale.ROOT FQCN with a
  Locale import. Same convention.

Skipped findings (not actionable as stated):
- "RoundTripTest doesn't structurally compare JSON" — actually does;
  assertRoundTrip uses JsonUtils.objectToJsonNode and JsonNode.equals,
  which is structural, not byte-level.
- "applyJsonBlob masks Jackson exception detail" — the catch already
  attaches the cause AND embeds e.getMessage() (which carries the
  Jackson [Source: ...; line: N, column: N] suffix) in the
  DdlCompilationException message. The 400 response surfaces that.

Tests: 31 parser + 86 sql-ddl (+1 BOOLEAN round-trip) + 35 controller
DDL tests pass. Spotless / checkstyle / license clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Final-round review identified three actionable polish items.

- DdlExecutionResponse._dryRun: change primitive boolean to boxed
  Boolean so @JsonInclude(NON_NULL) elides it from SHOW TABLES /
  SHOW CREATE TABLE responses where dry-run semantics do not apply.
  Previously those responses always emitted "dryRun":false, which
  is misleading. CREATE and DROP responses still carry the field
  (both have a working dry-run path). Class Javadoc updated to
  document the per-operation visibility.

- PinotDdlRestletResource.executeCreate: expand the
  TableAlreadyExistsException race comment to acknowledge the
  second-order race where a third caller DROPs the table between
  the 409 throw and the IF-NOT-EXISTS re-check, causing a 409
  return even though IF NOT EXISTS would normally be satisfied.
  Recovery is a retry; closing the window requires a version-checked
  create-or-no-op primitive on PinotHelixResourceManager that does
  not currently exist.

- PinotDdlRestletResource.assertNoLogicalTableReferences: add
  Javadoc covering the O(L) ZK-read cost per DROP (L = cluster
  logical-table count). Matches the existing DELETE /tables/{name}
  contract; documented so bulk-drop callers know what to expect.

Tests: 35 controller DDL tests pass. Spotless / checkstyle / license
clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Before this commit, TIMESTAMP-as-time-column was covered at the
parser / compiler / emitter / round-trip layers but never at the
controller integration layer — the in-process layers exercise
DdlCompiler and CanonicalDdlEmitter in isolation but skip
PinotHelixResourceManager.addTable / addSchema and the validation
pipeline.

Add createWithTimestampTimeColumnRoundTrips to
PinotDdlRestletResourceTest that:
1. Issues CREATE TABLE with `ts TIMESTAMP DATETIME FORMAT
   '1:MILLISECONDS:TIMESTAMP' GRANULARITY '1:MILLISECONDS'`
   against a real ControllerTest cluster.
2. Verifies the persisted Schema's DateTimeFieldSpec normalizes the
   user-supplied format to the short token "TIMESTAMP" (the
   DateTimeFieldSpec rewrite the JSON API also performs).
3. Calls SHOW CREATE TABLE and asserts the emitted DDL renders
   "ts TIMESTAMP DATETIME FORMAT 'TIMESTAMP' GRANULARITY
   '1:MILLISECONDS'" — closing the round-trip loop at the wire
   level, not just the in-process emitter level.

Cleans up via DROP TABLE so subsequent tests are unaffected.

Tests: 18 integration tests pass (was 17).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DataType.BOOLEAN.convert("garbage") returns 0 via BooleanUtils.toInt
rather than throwing — non-true/false strings map silently to false.
This matches the JSON /tables endpoint with
`"defaultNullValue": "<garbage>"` so DDL behavior is consistent with
the rest of Pinot, but the silent coercion is non-obvious to a reader
of DdlCompiler.toFieldSpec who would expect convert() to throw the
same way INT does for non-numeric strings.

Add a "Caveat for BOOLEAN" note where the validation lives so the
behavior is locally discoverable. No semantic change.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces SQL DDL support to Apache Pinot via a single POST /sql/ddl controller endpoint. Adds a new Calcite grammar extension and AST nodes in pinot-common, a new pinot-sql-ddl module that compiles AST → (Schema, TableConfig) and reverse-emits canonical DDL for SHOW CREATE TABLE, and wires the endpoint into pinot-controller reusing the existing validation/persistence pipeline. Existing /sql endpoint is updated to reject DDL with a clear redirect.

Changes:

  • New pinot-sql-ddl module with DdlCompiler, PropertyMapping (forward routing of PROPERTIES entries to promoted scalars / stream / task / custom configs / JSON blobs) and CanonicalDdlEmitter (reverse path with shadow-key rejection, deterministic property ordering, BIG_DECIMAL/BYTES/BOOLEAN/TIMESTAMP-safe default emission).
  • Grammar/AST additions in pinot-common (config.fmpp, parserImpls.ftl, 6 SqlPinot* nodes) and DDL classification in CalciteSqlParser.
  • New PinotDdlRestletResource* controller resource with hybrid-pair schema compatibility check (describeColumnShapeMismatch), DDL-on-/sql rejection in PinotQueryResource, and DTOs DdlExecutionRequest/DdlExecutionResponse.

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pom.xml, pinot-controller/pom.xml, pinot-sql-ddl/pom.xml Register the new module and wire it into pinot-controller.
pinot-sql-ddl/README.md, package-info.java Module overview and exception → HTTP-status contract docs.
pinot-common/src/main/codegen/config.fmpp Adds DDL keywords as non-reserved tokens and registers the new statement parser methods.
pinot-common/src/main/codegen/includes/parserImpls.ftl JavaCC productions for CREATE/DROP TABLE, SHOW TABLES, SHOW CREATE TABLE, with PRIMARY KEY, PROPERTIES, and DATETIME FORMAT … GRANULARITY ….
pinot-common/.../sql/parsers/parser/SqlPinot*.java New AST nodes (CreateTable, DropTable, ShowTables, ShowCreateTable, ColumnDeclaration, Property).
pinot-common/.../CalciteSqlParser.java Recognizes the new SqlPinot* nodes and classifies them as PinotSqlType.DDL; touches up some Javadoc to /// style.
pinot-common/.../PinotDdlParserTest.java 31 parser tests (syntax, IF EXISTS variants, identifier-as-keyword, PRIMARY KEY error path).
pinot-sql-ddl/.../compile/*.java Forward compiler: DdlCompiler, PropertyMapping, DataTypeMapper, exception type, compiled-result hierarchy.
pinot-sql-ddl/.../resolved/*.java Immutable typed IR between AST and compiled artifacts.
pinot-sql-ddl/.../reverse/*.java Reverse emitter: CanonicalDdlEmitter, SchemaEmitter, PropertyExtractor, SqlIdentifiers.
pinot-sql-ddl/src/test/.../{compile,reverse}/*.java 41 compiler and 21 emitter golden-output tests.
pinot-controller/.../ddl/DdlExecutionRequest.java, DdlExecutionResponse.java REST DTOs with operation-conditional field inclusion.
pinot-controller/.../PinotQueryResource.java Returns a clear validation error directing DDL callers to /sql/ddl.
pinot-controller/.../PinotQueryResourceTest.java Three new tests covering the DDL-on-/sql redirect across query-option permutations.
pinot-controller/.../PinotDdlRestletResourceUnitTest.java 18 unit tests for the column-shape comparator and DROP guards (logical-table refs, post-preflight task race, DB-header conflicts).
pinot-controller/.../PinotDdlRestletResourceTest.java 17 end-to-end controller integration tests on a real ControllerTest cluster.

@xiangfu0 xiangfu0 merged commit 2ffaa1c into apache:master May 16, 2026
13 checks passed
@xiangfu0 xiangfu0 deleted the feature/sql-ddl-slices-1-2 branch May 16, 2026 00:47
@xiangfu0
Copy link
Copy Markdown
Contributor Author

Opened docs PR pinot-contrib/pinot-docs#807 to document the new SQL table DDL endpoint () and its Querying & SQL / controller API docs surfaces.

@xiangfu0
Copy link
Copy Markdown
Contributor Author

Opened docs PR pinot-contrib/pinot-docs#807 to document the new SQL table DDL endpoint (POST /sql/ddl) and its Querying & SQL / controller API docs surfaces.

xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request May 16, 2026
## Summary

- add a dedicated Querying & SQL page for controller-managed SQL table
DDL
- wire the new page into Querying & SQL navigation and clarify the SQL
syntax/reference pages
- document `POST /sql/ddl` in the controller API examples page

## What changed for readers

Readers now have a single docs path for `CREATE TABLE`, `DROP TABLE`,
`SHOW TABLES`, and `SHOW CREATE TABLE`, including the controller
endpoint contract and the distinction between controller DDL and broker
query execution.

## Structural changes

- added `build-with-pinot/querying-and-sql/sql-ddl.md`
- added navigation links from `SUMMARY.md` and
`build-with-pinot/querying-and-sql/README.md`
- added discoverability notes in `sql-syntax.md` and `sql-reference.md`
- added `POST /sql/ddl` coverage in
`reference/api-reference/controller-api.md`

## Cross-checks against apache/pinot

- verified the controller endpoint contract in
`pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotDdlRestletResource.java`
- verified supported statement shapes in
`pinot-common/src/test/java/org/apache/pinot/sql/parsers/PinotDdlParserTest.java`
- verified create/show/dry-run behavior in
`pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotDdlRestletResourceTest.java`

## Validation

- `git diff --check`
- resolved relative links and `content-ref` targets for all edited docs
files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql-compliance Related to SQL standard compliance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants