Skip to content

feat(plugin-prometheus): Enable case-sensitive identifier support for Prometheus connector#26260

Merged
Dilli-Babu-Godari merged 1 commit into
prestodb:masterfrom
Dilli-Babu-Godari:prometheus_mixedcase_revised
May 14, 2026
Merged

feat(plugin-prometheus): Enable case-sensitive identifier support for Prometheus connector#26260
Dilli-Babu-Godari merged 1 commit into
prestodb:masterfrom
Dilli-Babu-Godari:prometheus_mixedcase_revised

Conversation

@Dilli-Babu-Godari

@Dilli-Babu-Godari Dilli-Babu-Godari commented Oct 9, 2025

Copy link
Copy Markdown
Contributor

Description

Added catalog-level config to support case-sensitive-name-matching

This PR implements case sensitivity handling for Prometheus metric (table) names:

1. Added case-sensitive name matching support controlled by the case-sensitive-name-matching configuration property
2. Implemented normalizeIdentifier in PrometheusMetadata to respect case sensitivity settings
3. Modified PrometheusClient to preserve case when case-sensitive matching is enabled

When case-sensitive-name-matching is set to true:

1. Metric (table) names will preserve their original case
2. Queries must use the exact case as defined in Prometheus

When case-sensitive-name-matching is set to false (default for backward compatibility):

1. All names are normalized to lowercase
2. Queries are case-insensitive

Note:

These three columns (labels, timestamp, value) are the fixed for all Prometheus metric tables. They represent:

labels: Map of label key-value pairs from Prometheus metrics
timestamp: Time when the metric was recorded
value: The numeric value of the metric

So the reason we have case-sensitive identifier support is extended to only tables but not to columns.

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==
Prometheus Connector Changes
* Add mixed case-sensitive identifier support for Prometheus connector.

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Oct 9, 2025
@prestodb-ci prestodb-ci requested review from a team, Mariamalmesfer and jkhaliqi and removed request for a team October 9, 2025 11:06
@sourcery-ai

sourcery-ai Bot commented Oct 9, 2025

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Introduce an optional case-sensitive identifier matching mode for Prometheus connector by extending configuration, updating metadata resolution logic to normalize and compare table names based on the mode, and enriching tests to cover both default and case-sensitive behaviors.

Class diagram for updated PrometheusConnectorConfig and PrometheusMetadata

classDiagram
    class PrometheusConnectorConfig {
        URI prometheusURI
        String trustStorePath
        String truststorePassword
        boolean verifyHostName
        boolean caseSensitiveNameMatchingEnabled
        +getPrometheusURI()
        +setVerifyHostName(boolean)
        +isCaseSensitiveNameMatchingEnabled()
        +setCaseSensitiveNameMatchingEnabled(boolean)
    }
    class PrometheusMetadata {
        PrometheusClient prometheusClient
        boolean caseSensitiveNameMatchingEnabled
        +normalizeIdentifier(ConnectorSession, String)
    }
    PrometheusMetadata --> PrometheusConnectorConfig : uses config
    PrometheusConnectorConfig <|-- PrometheusConnectorConfig : config property added
Loading

File-Level Changes

Change Details Files
Add case-sensitive-name-matching configuration flag
  • Introduce boolean field in PrometheusConnectorConfig
  • Add setter with @config and getter
  • Include new flag in default and explicit property mappings in tests
PrometheusConnectorConfig.java
TestPrometheusConnectorConfig.java
Propagate config flag to metadata and implement normalization
  • Extend PrometheusMetadata constructor to accept config
  • Store caseSensitiveNameMatchingEnabled flag
  • Implement normalizeIdentifier honoring the flag
  • Use Locale.ROOT for lowercase conversion
PrometheusMetadata.java
Enhance table lookup logic for case-sensitive matching
  • Replace single getTable calls with iteration over getTableNames
  • Compare normalized requested vs actual names
  • Return handle or metadata only on match
  • Return null when no match
PrometheusMetadata.java
Support passing extra connector properties in tests
  • Add overloaded createPrometheusQueryRunner with extraProperties map
  • Merge extraProperties into default properties
PrometheusQueryRunner.java
Update existing tests to use new metadata constructor
  • Pass config instance to PrometheusMetadata in all tests
  • Ensure default flag behavior is preserved
PrometheusQueryRunner.java
TestPrometheusRetrieveUpValueIntegrationTests.java
Add integration test for mixed-case matching
  • Implement TestPrometheusMixedCase extending AbstractTestQueryFramework
  • Configure runner with case-sensitive property
  • Validate success and failure of queries with correct and incorrect case
TestPrometheusMixedCase.java

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot 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.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `presto-prometheus/src/test/java/com/facebook/presto/plugin/prometheus/TestPrometheusMixedCase.java:63-64` </location>
<code_context>
+                .setCaseSensitiveNameMatchingEnabled(false));
     }

     @Test
</code_context>

<issue_to_address>
**suggestion (testing):** Missing test for case-insensitive mode with mixed-case table names.

Please add a test for the default case-insensitive mode to confirm that queries with varying table name casing (e.g., 'UP', 'Up', 'up') all work as expected.
</issue_to_address>

### Comment 2
<location> `presto-prometheus/src/test/java/com/facebook/presto/plugin/prometheus/TestPrometheusRetrieveUpValueIntegrationTests.java:57` </location>
<code_context>
     {
         this.server = new PrometheusServer();
         this.client = createPrometheusClient(server);
+        this.config = new PrometheusConnectorConfig();
     }

</code_context>

<issue_to_address>
**suggestion (testing):** Missing explicit tests for case sensitivity in PrometheusMetadata unit tests.

Please add unit tests for PrometheusMetadata with caseSensitiveNameMatchingEnabled set to both true and false, ensuring correct table handle resolution and metadata retrieval for mixed-case table names.

Suggested implementation:

```java
    private PrometheusServer server;
    private PrometheusClient client;
    private PrometheusConnectorConfig config;

    @BeforeClass
    protected void createQueryRunner()
    {
        this.server = new PrometheusServer();
        this.client = createPrometheusClient(server);
        this.config = new PrometheusConnectorConfig();
    }

    @Test
    public void testCaseSensitiveTableHandleResolution()
    {
        PrometheusConnectorConfig caseSensitiveConfig = new PrometheusConnectorConfig(true); // true for case sensitivity
        PrometheusMetadata metadata = new PrometheusMetadata(client, caseSensitiveConfig);

        // Assume "MixedCaseTable" exists in Prometheus
        SchemaTableName mixedCaseTable = new SchemaTableName("default", "MixedCaseTable");
        ConnectorTableHandle handle = metadata.getTableHandle(mixedCaseTable);

        assertNotNull(handle, "Table handle should be resolved for exact case match");

        // Try with different case
        SchemaTableName lowerCaseTable = new SchemaTableName("default", "mixedcasetable");
        ConnectorTableHandle lowerHandle = metadata.getTableHandle(lowerCaseTable);

        assertNull(lowerHandle, "Table handle should not be resolved for case mismatch when case sensitivity is enabled");
    }

    @Test
    public void testCaseInsensitiveTableHandleResolution()
    {
        PrometheusConnectorConfig caseInsensitiveConfig = new PrometheusConnectorConfig(false); // false for case insensitivity
        PrometheusMetadata metadata = new PrometheusMetadata(client, caseInsensitiveConfig);

        // Assume "MixedCaseTable" exists in Prometheus
        SchemaTableName mixedCaseTable = new SchemaTableName("default", "MixedCaseTable");
        ConnectorTableHandle handle = metadata.getTableHandle(mixedCaseTable);

        assertNotNull(handle, "Table handle should be resolved for exact case match");

        // Try with different case
        SchemaTableName lowerCaseTable = new SchemaTableName("default", "mixedcasetable");
        ConnectorTableHandle lowerHandle = metadata.getTableHandle(lowerCaseTable);

        assertNotNull(lowerHandle, "Table handle should be resolved for case mismatch when case sensitivity is disabled");
    }

```

- Ensure that `PrometheusConnectorConfig` has a constructor that accepts a boolean for `caseSensitiveNameMatchingEnabled`.
- Make sure that the Prometheus test server contains a table named "MixedCaseTable" for these tests to pass.
- If `PrometheusMetadata` or `PrometheusConnectorConfig` do not support these options, you will need to add them.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch 3 times, most recently from b5d3322 to ef3ff6c Compare October 9, 2025 12:55

@steveburnett steveburnett 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.

Thanks for the doc! Only a few minor comments, overall it looks good.

Comment thread presto-docs/src/main/sphinx/connector/prometheus.rst Outdated
Comment thread presto-docs/src/main/sphinx/connector/prometheus.rst Outdated
Comment thread presto-docs/src/main/sphinx/connector/prometheus.rst Outdated
Comment thread presto-docs/src/main/sphinx/connector/prometheus.rst Outdated
@steveburnett

Copy link
Copy Markdown
Contributor

Thanks for the release note! Nits:

== RELEASE NOTES ==
Prometheus Connector Changes
* Add mixed case-sensitive identifier support for Prometheus connector.

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch 3 times, most recently from 5f71407 to 147b24e Compare October 9, 2025 15:29
@Dilli-Babu-Godari

Copy link
Copy Markdown
Contributor Author

Thanks for the release note! Nits:

== RELEASE NOTES ==
Prometheus Connector Changes
* Add mixed case-sensitive identifier support for Prometheus connector.

Thanks for the review! I’ve addressed all the comments.

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch 2 times, most recently from ba85ae8 to 6ccf6bf Compare October 9, 2025 17:22
steveburnett
steveburnett previously approved these changes Oct 9, 2025

@steveburnett steveburnett 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.

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

Comment thread presto-docs/src/main/sphinx/connector/prometheus.rst Outdated
@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@prestodb-ci prestodb-ci added follow-up-4 4th time follow-up (alchemy generated) and removed follow-up-3 3rd time follow-up (alchemy generated) labels Apr 2, 2026
@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch from cc21463 to 4b005e7 Compare April 6, 2026 17:40
@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch from 4b005e7 to 85cde9f Compare April 6, 2026 18:17
@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@agrawalreetika agrawalreetika left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR. Let few comments.
Also, please correct typo in the commit message,

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the prometheus_mixedcase_revised branch from 85cde9f to ef6e26d Compare May 14, 2026 08:54
@prestodb-ci

Copy link
Copy Markdown
Contributor

Assigning this issue to @faizdani because you are the default assignee for issue follow-up scheme ForwardFit. Feel free to re-route accordingly.

@prestodb-ci

Copy link
Copy Markdown
Contributor

@faizdani
Can you please provide an update on this issue? Thank you!

@Dilli-Babu-Godari Dilli-Babu-Godari merged commit bf30efc into prestodb:master May 14, 2026
124 of 131 checks passed
msmygit pushed a commit to msmygit/presto that referenced this pull request Jun 3, 2026
… Prometheus connector (prestodb#26260)

## Description
Added catalog-level config to support case-sensitive-name-matching

This PR implements case sensitivity handling for Prometheus metric
(table) names:

1. Added case-sensitive name matching support controlled by the
case-sensitive-name-matching configuration property
2. Implemented normalizeIdentifier in PrometheusMetadata to respect case
sensitivity settings
3. Modified PrometheusClient to preserve case when case-sensitive
matching is enabled
When case-sensitive-name-matching is set to true:

    1. Metric (table) names will preserve their original case
    2. Queries must use the exact case as defined in Prometheus
When case-sensitive-name-matching is set to false (default for backward
compatibility):

    1. All names are normalized to lowercase
    2. Queries are case-insensitive

**Note:** 


https://github.com/prestodb/presto/blob/76d8bf2f42476c36e429e7515797f452ca79aef3/presto-prometheus/src/main/java/com/facebook/presto/plugin/prometheus/PrometheusClient.java#L148


https://github.com/prestodb/presto/blob/76d8bf2f42476c36e429e7515797f452ca79aef3/presto-prometheus/src/main/java/com/facebook/presto/plugin/prometheus/PrometheusClient.java#L151

These three columns (labels, timestamp, value) are the fixed for all
Prometheus metric tables. They represent:

_labels_: Map of label key-value pairs from Prometheus metrics
_timestamp_: Time when the metric was recorded
_value_: The numeric value of the metric

So the reason we have case-sensitive identifier support is extended to
only tables but not to columns.

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## Contributor checklist

- [ ] Please make sure your submission complies with our [contributing
guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md),
in particular [code
style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style)
and [commit
standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards).
- [ ] PR description addresses the issue accurately and concisely. If
the change is non-trivial, a GitHub Issue is referenced.
- [ ] Documented new properties (with its default value), SQL syntax,
functions, or other functionality.
- [ ] If release notes are required, they follow the [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines).
- [ ] Adequate tests were added if applicable.
- [ ] CI passed.

## Release Notes
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==
Prometheus Connector Changes
* Add mixed case-sensitive identifier support for Prometheus connector.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

follow-up-4 4th time follow-up (alchemy generated) ForwardFit Items from IBM Forward Fit from:IBM PR from IBM need-follow-up Need any type of follow-up (alchemy generated)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants