fix(plugin-pinot): Normalize mixed-case Pinot table/column name handling when case-sensitive flag is disabled #26663
+23
−12
Conversation
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
…ing when case-sensitive flag is disabled
Contributor
Reviewer's GuideNormalizes Pinot table and column identifier handling based on the connector’s case-sensitivity flag, ensuring mixed-case names work correctly when case-sensitive matching is disabled, and threads ConnectorSession and configuration through the relevant metadata and schema utilities. Sequence diagram for Pinot table and column resolution with case-sensitivity flagsequenceDiagram
actor User
participant PrestoEngine
participant PinotMetadata
participant PinotConnection
participant PinotClusterInfoFetcher
participant PinotColumnUtils
User->>PrestoEngine: Submit query (tableName, columnNames)
PrestoEngine->>PinotMetadata: getTableHandle(session, schemaTableName)
PinotMetadata->>PinotMetadata: getPinotTableNameFromPrestoTableName(session, prestoTableName)
PinotMetadata->>PinotConnection: getTableNames()
PinotConnection-->>PinotMetadata: allTables
PinotMetadata->>PinotMetadata: normalizedPrestoTableName = normalizeIdentifier(session, prestoTableName)
loop for each pinotTableName in allTables
PinotMetadata->>PinotMetadata: normalizedPinotTableName = normalizeIdentifier(session, pinotTableName)
PinotMetadata-->>PinotMetadata: compare normalizedPrestoTableName and normalizedPinotTableName
end
PinotMetadata-->>PrestoEngine: PinotTableHandle(connectorId, schemaName, resolvedPinotTableName)
PrestoEngine->>PinotMetadata: getColumnHandles(session, tableHandle)
PinotMetadata->>PinotMetadata: getPinotTableNameFromPrestoTableName(session, pinotTableHandle.tableName)
PinotMetadata->>PinotConnection: getTable(resolvedPinotTableName)
PinotConnection-->>PinotMetadata: PinotTable
PrestoEngine->>PinotConnection: load(resolvedPinotTableName)
PinotConnection->>PinotClusterInfoFetcher: getTableSchema(resolvedPinotTableName)
PinotClusterInfoFetcher-->>PinotConnection: Schema tablePinotSchema
alt pinotConfig.isCaseSensitiveNameMatchingEnabled == true
PinotConnection->>PinotColumnUtils: getPinotColumnsForPinotSchema(schema, inferDateType, inferTimestampType, nullHandlingEnabled, true)
PinotColumnUtils-->>PinotConnection: PinotColumn list (original columnName)
else pinotConfig.isCaseSensitiveNameMatchingEnabled == false
PinotConnection->>PinotColumnUtils: getPinotColumnsForPinotSchema(schema, inferDateType, inferTimestampType, nullHandlingEnabled, false)
PinotColumnUtils-->>PinotConnection: PinotColumn list (lowercased columnName)
end
PinotConnection-->>PrestoEngine: PinotColumn list
PrestoEngine-->>User: Query planned and executed with normalized identifiers
Updated class diagram for PinotMetadata, PinotConnection, and PinotColumnUtilsclassDiagram
class PinotMetadata {
- String connectorId
- PinotConnection pinotPrestoConnection
+ List~String~ listSchemaNames(ConnectorSession session)
- String getPinotTableNameFromPrestoTableName(ConnectorSession session, String prestoTableName)
+ PinotTableHandle getTableHandle(ConnectorSession session, SchemaTableName tableName)
+ ConnectorTableMetadata getTableMetadata(ConnectorSession session, ConnectorTableHandle table)
+ Map~String, ColumnHandle~ getColumnHandles(ConnectorSession session, ConnectorTableHandle tableHandle)
+ Map~SchemaTableName, List~ColumnMetadata~~ listTableColumns(ConnectorSession session, SchemaTablePrefix prefix)
- ConnectorTableMetadata getTableMetadata(ConnectorSession session, SchemaTableName tableName)
- String normalizeIdentifier(ConnectorSession session, String identifier)
}
class PinotConnection {
- PinotClusterInfoFetcher pinotClusterInfoFetcher
- PinotConfig pinotConfig
- Executor executor
- boolean nullHandlingEnabled
+ List~String~ getTableNames()
+ PinotTable getTable(String tableName)
+ List~PinotColumn~ load(String tableName)
}
class PinotColumnUtils {
<<utility>>
- PinotColumnUtils()
+ List~PinotColumn~ getPinotColumnsForPinotSchema(Schema pinotTableSchema, boolean inferDateType, boolean inferTimestampType)
+ List~PinotColumn~ getPinotColumnsForPinotSchema(Schema pinotTableSchema, boolean inferDateType, boolean inferTimestampType, boolean nullHandlingEnabled, boolean isCaseSensitiveNameMatchingEnabled)
}
class PinotConfig {
+ boolean isInferDateTypeInSchema()
+ boolean isInferTimestampTypeInSchema()
+ boolean isCaseSensitiveNameMatchingEnabled()
}
class Schema {
+ List~String~ getColumnNames()
+ FieldSpec getFieldSpecFor(String columnName)
}
class PinotColumn {
+ String columnName
+ Type prestoType
+ boolean nullable
+ String comment
}
class PinotTable {
+ String tableName
}
class PinotClusterInfoFetcher {
+ Schema getTableSchema(String tableName)
}
class ConnectorSession
class SchemaTableName {
+ String getSchemaName()
+ String getTableName()
}
class ConnectorTableHandle
class PinotTableHandle {
+ String getConnectorId()
+ String getSchemaName()
+ String getTableName()
+ SchemaTableName toSchemaTableName()
}
class ColumnHandle
class ColumnMetadata
class SchemaTablePrefix
class Type
class FieldSpec
class Executor
PinotMetadata --> PinotConnection : uses
PinotMetadata --> PinotTableHandle : creates
PinotMetadata --> ConnectorSession : uses
PinotMetadata --> SchemaTableName : uses
PinotMetadata --> ConnectorTableMetadata : returns
PinotMetadata --> ColumnHandle : returns
PinotMetadata --> ColumnMetadata : returns
PinotConnection --> PinotClusterInfoFetcher : uses
PinotConnection --> PinotConfig : uses
PinotConnection --> Executor : uses
PinotConnection --> PinotColumn : returns
PinotConnection --> PinotTable : returns
PinotColumnUtils --> Schema : uses
PinotColumnUtils --> PinotColumn : creates
PinotColumnUtils --> Type : uses
PinotColumnUtils --> FieldSpec : uses
PinotConfig --> boolean
Schema --> FieldSpec
PinotTableHandle --|> ConnectorTableHandle
ConnectorSession --> PinotConfig : resolves case sensitivity
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
prestodb-ci
requested review from
a team,
pdabre12 and
pramodsatya
and removed request for
a team
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider reusing the same normalization helper used for table names (e.g.,
normalizeIdentifieror a shared utility) inPinotColumnUtilsinstead of manually callingcolumnName.toLowerCase(Locale.ROOT)so that table and column name normalization stay consistent and driven by the same logic/configuration. - In
getPinotTableNameFromPrestoTableName, you now normalize both the Presto and Pinot table names; if multiple Pinot tables differ only by case when case-insensitive matching is configured, you may want to explicitly define or document the tie-breaking behavior (e.g., which one is chosen) rather than implicitly returning the first match.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider reusing the same normalization helper used for table names (e.g., `normalizeIdentifier` or a shared utility) in `PinotColumnUtils` instead of manually calling `columnName.toLowerCase(Locale.ROOT)` so that table and column name normalization stay consistent and driven by the same logic/configuration.
- In `getPinotTableNameFromPrestoTableName`, you now normalize both the Presto and Pinot table names; if multiple Pinot tables differ only by case when case-insensitive matching is configured, you may want to explicitly define or document the tie-breaking behavior (e.g., which one is chosen) rather than implicitly returning the first match.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Description
This bug fix normalizes identifiers for tables and columns when the case-sensitive flag is disabled. As a result, mixed case table/column names no longer cause query failures when using the Pinot connector in Presto.
Motivation and Context
to fix failing queries when the flag is disabled
Impact
Test Plan
Tested using a Pinot connector instance from IBM QA. Also tested with the flag enabled to ensure behavior remains unchanged.
Flag disabled;
Flag enabled;
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.