Skip to content

Conversation

ekaterinadimitrova2
Copy link

What is the issue

...
We need that knowledge for CNDB

What does this PR fix and why was it fixed

...
It exposes containsDateRangeTypeColumn methods

Copy link

github-actions bot commented Jul 24, 2025

Checklist before you submit for review

  • Make sure there is a PR in the CNDB project updating the Converged Cassandra version
  • Use NoSpamLogger for log lines that may appear frequently in the logs
  • Verify test results on Butler
  • Test coverage for new/modified code is > 80%
  • Proper code formatting
  • Proper title for each commit staring with the project-issue number, like CNDB-1234
  • Each commit has a meaningful description
  • Each commit is not very long and contains related changes
  • Renames, moves and reformatting are in distinct commits
  • All new files should contain the DataStax copyright header instead of the Apache License one

@ekaterinadimitrova2
Copy link
Author

@pkolaczk , I just addressed your comment.

Also, for completeness, I want to mention that I am intentionally leaving the testing of the new methods to CNDB.

Copy link

@Copilot 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

Exposes containsDateRangeTypeColumn methods to support CNDB functionality for detecting date range column types. This enhancement provides visibility into schema components that contain DateRangeType columns.

Key changes:

  • Added public method to check for date range columns in table creation and alteration statements
  • Implemented date range type detection in CQL3Type system

Reviewed Changes

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

File Description
CreateTableStatement.java Added public method to detect DateRangeType columns during table creation
AlterTableStatement.java Added base method and override in AddColumns inner class for date range detection
CQL3Type.java Added isDateRange() method with implementation for custom types and spelling correction

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@Copilot 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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ekaterinadimitrova2
Copy link
Author

Neither of the two failures flagged as regressions is related to the changes done here.

Copy link

@maxtomassi maxtomassi left a comment

Choose a reason for hiding this comment

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

Just a suggestion to avoid the use of hardcoded string name. Looks good otherwise.

@ekaterinadimitrova2
Copy link
Author

Just a suggestion to avoid the use of hardcoded string name. Looks good otherwise.

Suggestion committed and branch rebased for final CI test, thanks!

Copy link

sonarqubecloud bot commented Oct 1, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@cassci-bot
Copy link

❌ Build ds-cassandra-pr-gate/PR-1914 rejected by Butler


1 regressions found
See build details here


Found 1 new test failures

Test Explanation Runs Upstream
o.a.c.index.sai.cql.VectorSiftSmallTest.testSiftSmall (compression) <span title="The test is FLAKY on feature and PASSING on upstream:
• Feature branch: FLAKY because there are 1 passing, 1 failing, 0 missing, and 0 skipped runs in the last 2 runs in 4 builds (2 to 7)
• Upstream branch: PASSING in the last 7 runs in 7 builds (1578 to 1587)">REGRESSION 🔵🔴 0 / 7

Found 2 known test failures

@ekaterinadimitrova2
Copy link
Author

testSiftSmall does not fail for me locally and it doesn't seem to be touching anything we touch here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants