Skip to content

Rename PinotDataType.INTEGER to INT for naming consistency#18815

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:cleanup-rename-pinot-data-type-int
Jun 21, 2026
Merged

Rename PinotDataType.INTEGER to INT for naming consistency#18815
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:cleanup-rename-pinot-data-type-int

Conversation

@Jackie-Jiang

Copy link
Copy Markdown
Contributor

Summary

Renames the enum constants PinotDataType.INTEGERINT and INTEGER_ARRAYINT_ARRAY so the spelling matches FieldSpec.DataType.INT, DataSchema.ColumnDataType.INT, and the SQL type name. The mismatch (INTEGER vs the INT everyone else uses) is a recurring source of confusion.

PinotDataType is a transient type-conversion helper — it is never persisted by name into segment metadata, ZooKeeper, table config, or the wire protocol (only FieldSpec.DataType is, and it is untouched). So the rename carries no serialization or mixed-version compatibility risk.

Backward compatibility

The only place that resolves a PinotDataType from a user-supplied string by name is the cast scalar function. CAST(x AS INTEGER) previously resolved via the PinotDataType.valueOf("INTEGER") default branch. An explicit case "INTEGER" alias is added so that literal keeps working (alongside INT), and a regression test pins it.

The rest of the change is a mechanical reference update (PinotDataType.INTEGERINT, case INTEGER:case INT: for PinotDataType switches only — Calcite SqlTypeName/SqlTypeFamily and java.sql.Types INTEGER references are left intact) plus test-data updates.

Rename the enum constants `PinotDataType.INTEGER` -> `INT` and `INTEGER_ARRAY` ->
`INT_ARRAY` so the spelling matches `FieldSpec.DataType.INT`, `ColumnDataType.INT`,
and the SQL type name. `PinotDataType` is a transient conversion helper and is never
persisted by name into segment metadata, ZooKeeper, table config, or the wire protocol,
so the rename carries no serialization / mixed-version compatibility risk.

The only name-based reader is the `cast` scalar function, which accepts a type literal:
`CAST(x AS INTEGER)` previously resolved via `PinotDataType.valueOf("INTEGER")`. An
explicit `case "INTEGER"` alias is added so that literal keeps working (alongside `INT`).
@Jackie-Jiang Jackie-Jiang added refactor Code restructuring without changing behavior backward-incompat Introduces a backward-incompatible API or behavior change labels Jun 19, 2026
@Jackie-Jiang Jackie-Jiang requested a review from Copilot June 20, 2026 00:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR renames the PinotDataType enum constants INTEGERINT and INTEGER_ARRAYINT_ARRAY to align naming with the rest of Pinot (FieldSpec.DataType.INT, DataSchema.ColumnDataType.INT, and SQL type naming). It also preserves query compatibility for CAST(x AS INTEGER) by explicitly aliasing "INTEGER" to PinotDataType.INT in the cast implementation and adds a regression test.

Changes:

  • Rename PinotDataType.INTEGER/INTEGER_ARRAY to INT/INT_ARRAY and update internal call sites/switches accordingly.
  • Update CAST/type-conversion handling to keep "INTEGER" working as a cast literal (mapped to INT), and add a test to pin this behavior.
  • Update single-stage query tests and other unit tests to use INT in expected PinotDataType-derived type headers.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pinot-spi/src/test/java/org/apache/pinot/spi/utils/PinotDataTypeTest.java Updates unit tests to reference INT/INT_ARRAY after enum rename.
pinot-spi/src/main/java/org/apache/pinot/spi/utils/PinotDataType.java Renames enum constants and updates conversion/type-inference logic to use INT/INT_ARRAY.
pinot-spi/src/main/java/org/apache/pinot/spi/data/OpenStructTypeInference.java Updates switch case to match PinotDataType.INT for inferring FieldSpec.DataType.INT.
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java Updates PinotDataTypeFieldSpec.DataType mapping for int-like types to use INT.
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/columntransformer/DataTypeColumnTransformer.java Updates no-op transformation checks to compare against PinotDataType.INT/INT_ARRAY.
pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOrderByTest.java Updates fluent test header types from INTEGER to INT (PinotDataType header parsing).
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/ModeAggregationFunctionTest.java Simplifies PinotDataType lookup now that FieldSpec.DataType.INT.name() maps to PinotDataType.INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinMaxRangeAggregationFunctionTest.java Same as above: relies on PinotDataType.valueOf(dataType.name()) for INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/LastWithTimeAggregationFunctionTest.java Same as above: uses PinotDataType.valueOf(dataType.name()).
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/FirstWithTimeAggregationFunctionTest.java Same as above: uses PinotDataType.valueOf(dataType.name()).
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctSumAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapMVAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctCountAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctAvgAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/DistinctAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/CountAggregationFunctionTest.java Updates fluent test header types to INT.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AvgAggregationFunctionTest.java Updates fluent test header types to INT in expectations.
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java Updates literal/non-literal argument conversions and switch cases to use PinotDataType.INT.
pinot-common/src/test/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctionsTest.java Adds regression test ensuring CAST(... AS INTEGER) continues to work (alias to INT).
pinot-common/src/test/java/org/apache/pinot/common/function/FunctionUtilsTest.java Updates expected parameter/argument PinotDataTypes for int/Integer and Integer[] to INT/INT_ARRAY.
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java Updates ColumnDataType.INTPinotDataType.INT mapping.
pinot-common/src/main/java/org/apache/pinot/common/request/context/LiteralContext.java Updates literal pinot type resolution for INT to PinotDataType.INT.
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java Adds "INTEGER" cast literal mapping to PinotDataType.INT and updates INT comparisons accordingly.
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionUtils.java Updates parameter type mapping for int/Integer to PinotDataType.INT.

Comment on lines +65 to 69
case "INTEGER":
case "UTINYINT":
case "USMALLINT":
targetDataType = INTEGER;
targetDataType = INT;
break;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This toUpperCase() is pre-existing and unrelated to the rename, so I'm keeping this PR scoped to the rename rather than mixing in a behavior fix. Agree it should use Locale.ROOT — I'll address the locale-safe normalization in a separate PR.

},

INTEGER {
INT {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PinotDataType is an internal type-conversion helper, not a persisted or wire-format type — only FieldSpec.DataType / DataSchema.ColumnDataType are serialized — so renaming a constant carries no data/format compatibility impact.

A @Deprecated alias isn't possible for an enum constant: INTEGER can't be made a synonym of INT, it would be a distinct constant with no value. Downstream code referencing PinotDataType.INTEGER directly needs a one-line update; the rename is intentional to align with FieldSpec.DataType.INT / ColumnDataType.INT / SQL naming.

@Jackie-Jiang Jackie-Jiang added the needs-attention Used for sensitive changes - allows searching PRs post release to narrow down causes for regression. label Jun 20, 2026
@codecov-commenter

codecov-commenter commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.80%. Comparing base (648d285) to head (1cbf7c4).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...java/org/apache/pinot/spi/utils/PinotDataType.java 72.72% 3 Missing ⚠️
...e/pinot/common/request/context/LiteralContext.java 50.00% 0 Missing and 1 partial ⚠️
...l/columntransformer/DataTypeColumnTransformer.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #18815   +/-   ##
=========================================
  Coverage     64.80%   64.80%           
  Complexity     1319     1319           
=========================================
  Files          3388     3388           
  Lines        210228   210228           
  Branches      32948    32948           
=========================================
+ Hits         136229   136235    +6     
+ Misses        63028    63022    -6     
  Partials      10971    10971           
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.80% <77.27%> (+<0.01%) ⬆️
temurin 64.80% <77.27%> (+<0.01%) ⬆️
unittests 64.80% <77.27%> (+<0.01%) ⬆️
unittests1 56.98% <72.72%> (-0.01%) ⬇️
unittests2 37.26% <45.45%> (+<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.

@Jackie-Jiang Jackie-Jiang merged commit dd4fcf5 into apache:master Jun 21, 2026
17 of 21 checks passed
@Jackie-Jiang Jackie-Jiang deleted the cleanup-rename-pinot-data-type-int branch June 21, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Introduces a backward-incompatible API or behavior change needs-attention Used for sensitive changes - allows searching PRs post release to narrow down causes for regression. refactor Code restructuring without changing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants