Skip to content

CNDB-12683 validate table name length for not-internal #1623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Mar 12, 2025
Merged

CNDB-12683 validate table name length for not-internal #1623

merged 6 commits into from
Mar 12, 2025

Conversation

@k-rus
Copy link
Member

@k-rus k-rus commented Mar 7, 2025 •
edited
Loading

What is the issue

Fixes https://github.com/riptano/cndb/issues/12683

Table names are used in file names. Since the table names were not validated on its length, creating or flushing a table with too long name fails on too long file name.
There are two cases with using table names in file names:

  • keyspace name . table name -controller-config.JSON
  • table name - 32 chars table id

The maximum allowed file name size is 255 chars. Thus,

  • keyspace and table name, which are together longer than 231 chars, will fail.
  • a table name, which is longer than 222 chars, will fail.

What does this PR fix and why was it fixed

This PR checks that creating new table name should not have too long table name.
New tables are identified through the query's client state, which should not be internal.
The limit is 222 chars for combined keyspace and table names. This is more restrictive than necessary, but is easier to explain in the documentation.

The change is tested in CNDB that creating new tables with long names are prevented, but existing tables still work.

@github-actions
Copy link

github-actions bot commented Mar 7, 2025 •
edited by k-rus
Loading

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

@k-rus k-rus force-pushed the rf-cndb-12683-validate-long-table-name branch from 405fd0f to 84118a9 Compare March 7, 2025 20:43
@k-rus k-rus force-pushed the rf-cndb-12683-validate-long-table-name branch from 320fa14 to d8b10aa Compare March 11, 2025 10:41
@k-rus k-rus changed the title [WIP] CNDB-12683 try to validate table name length for not-internal CNDB-12683 try to validate table name length for not-internal Mar 12, 2025
@k-rus k-rus self-assigned this Mar 12, 2025
@k-rus k-rus requested review from a team March 12, 2025 15:05
@k-rus k-rus changed the title CNDB-12683 try to validate table name length for not-internal CNDB-12683 validate table name length for not-internal Mar 12, 2025
eolivelli
eolivelli approved these changes Mar 12, 2025
View reviewed changes
Copy link

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@k-rus
Copy link
Member Author

k-rus commented Mar 12, 2025 •
edited
Loading

I changed internal validation of keyspace name during table create.

The length of keyspace name is validated when created with CREATE KEYSPACE statement. A validation of kyespace name also exists in CREATE TABLE statement, but checking the length was missing. My understanding it's needed when keyspace is created internally without validation, which happens in CNDB and some tests in CC. Thus my fix should be safe.

@k-rus k-rus force-pushed the rf-cndb-12683-validate-long-table-name branch from 2db7861 to 0d71dde Compare March 12, 2025 17:08
@k-rus
Copy link
Member Author

k-rus commented Mar 12, 2025

@eolivelli I changed internal validation of keyspace name during table create.

The length of keyspace name is validated when created with CREATE KEYSPACE statement. A validation of kyespace name also exists in CREATE TABLE statement, but checking the length was missing. My understanding it's needed when keyspace is created internally without validation, which happens in CNDB and some tests in CC. Thus my fix should be safe.

I dropped this change from this PR and will do it after the release.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 12, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@cassci-bot
Copy link

cassci-bot commented Mar 12, 2025

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

1 new test failure(s) in 8 builds
See build details here

Found 1 new test failures

Test Explanation Branch history Upstream history
...testWithRangeTombstoneMarkersWithoutCompression regression 🔴🔵🔵🔵🔵🔴🔵 🔵🔵🔵🔵🔵🔵🔵

Found 217 known test failures

@k-rus k-rus merged commit e69c25f into main Mar 12, 2025
474 of 479 checks passed
@k-rus k-rus deleted the rf-cndb-12683-validate-long-table-name branch March 12, 2025 18:47
k-rus added a commit that referenced this pull request Mar 12, 2025
@k-rus
CNDB-12683 validate table name length for not-internal (#1623)
9021acd 
Fixes riptano/cndb#12683

Table names are used in file names. Since the table names were not
validated on its length, creating or flushing a table with too long name
fails on too long file name.
There are two cases with using table names in file names:
- keyspace_name .table_name-controller-config.JSON
- table_name-32chars_table_id 

The maximum allowed file name size is 255 chars. Thus, 
- keyspace and table names, which are together longer than 231 chars,
will fail.
- a table name, which is longer than 222 chars, will fail.

This PR checks that creating new table name should not have too long
table name.
New tables are identified through the query's client state, which should
not be internal.
The limit is 222 chars for combined keyspace and table names. This is
more restrictive than necessary, but is easier to explain in the
documentation.

The change is tested in CNDB that creating new tables with long names
are prevented, but existing tables still work.
k-rus added a commit that referenced this pull request Mar 13, 2025
@k-rus
CNDB-12683 validate table name length for not-internal (#1623) (#1629)
c50f637 
Fixes riptano/cndb#12683

Table names are used in file names. Since the table names were not
validated on its length, creating or flushing a table with too long name
fails on too long file name.
There are two cases with using table names in file names:
- keyspace_name .table_name-controller-config.JSON
- table_name-32chars_table_id 

The maximum allowed file name size is 255 chars. Thus, 
- keyspace and table names, which are together longer than 231 chars,
will fail.
- a table name, which is longer than 222 chars, will fail.

This PR checks that creating new table name should not have too long
table name.
New tables are identified through the query's client state, which should
not be internal.
The limit is 222 chars for combined keyspace and table names. This is
more restrictive than necessary, but is easier to explain in the
documentation.

The change is tested in CNDB that creating new tables with long names
are prevented, but existing tables still work.
djatnieks pushed a commit that referenced this pull request Apr 14, 2025
@k-rus @djatnieks
CNDB-12683 validate table name length for not-internal (#1623)
5a693ef 
Fixes riptano/cndb#12683

Table names are used in file names. Since the table names were not
validated on its length, creating or flushing a table with too long name
fails on too long file name.
There are two cases with using table names in file names:
- keyspace_name .table_name-controller-config.JSON
- table_name-32chars_table_id

The maximum allowed file name size is 255 chars. Thus,
- keyspace and table names, which are together longer than 231 chars,
will fail.
- a table name, which is longer than 222 chars, will fail.

This PR checks that creating new table name should not have too long
table name.
New tables are identified through the query's client state, which should
not be internal.
The limit is 222 chars for combined keyspace and table names. This is
more restrictive than necessary, but is easier to explain in the
documentation.

The change is tested in CNDB that creating new tables with long names
are prevented, but existing tables still work.
djatnieks pushed a commit that referenced this pull request May 18, 2025
@k-rus @djatnieks
CNDB-12683 validate table name length for not-internal (#1623)
8cc6812 
Fixes riptano/cndb#12683

Table names are used in file names. Since the table names were not
validated on its length, creating or flushing a table with too long name
fails on too long file name.
There are two cases with using table names in file names:
- keyspace_name .table_name-controller-config.JSON
- table_name-32chars_table_id

The maximum allowed file name size is 255 chars. Thus,
- keyspace and table names, which are together longer than 231 chars,
will fail.
- a table name, which is longer than 222 chars, will fail.

This PR checks that creating new table name should not have too long
table name.
New tables are identified through the query's client state, which should
not be internal.
The limit is 222 chars for combined keyspace and table names. This is
more restrictive than necessary, but is easier to explain in the
documentation.

The change is tested in CNDB that creating new tables with long names
are prevented, but existing tables still work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eolivelli eolivelli eolivelli approved these changes

Assignees

@k-rus k-rus

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@k-rus @cassci-bot @eolivelli