Skip to content

Conversation

@mykaul
Copy link

@mykaul mykaul commented Dec 23, 2025

With CoPilot, I removed the query, since this functionality doesn't exist in ScyllaDB at all.

Refs: #453
Refs: https://scylladb.atlassian.net/browse/CUSTOMER-62

Pre-review checklist

  • I have split my patch into logically separate commits.
  • [ x All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

With CoPilot, I removed the query, since this functionality doesn't exist in ScyllaDB at all.

Refs: scylladb#453
Refs: https://scylladb.atlassian.net/browse/CUSTOMER-62
Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
@mykaul
Copy link
Author

mykaul commented Dec 24, 2025

CI failures do not seem related to this patch :-/

Copy link

@Lorak-mmk Lorak-mmk left a comment

Choose a reason for hiding this comment

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

Can you try to run integration tests with Cassandra and see if there are new failures caused by this change?
We do not currently do this in CI, there was work to do this but it was not finished: #339
Let's not make things more difficult for someone picking it up again.

Did you consider using those queries only with Cassandra, instead of removing them completely?

indexes_result = self._handle_results(indexes_sucess, indexes_result, query_msg=indexes_query)
triggers_result = self._handle_results(triggers_success, triggers_result, query_msg=triggers_query)
return self._build_table_metadata(table_result[0], col_result, triggers_result, indexes_result)
return self._build_table_metadata(table_result[0], col_result, None, indexes_result)

Choose a reason for hiding this comment

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

Instead of passing None can you remove the argument from _build_table_metadata? Let's not introduce dead code.

table_name = row[self._table_name_col]

col_rows = col_rows or self.keyspace_table_col_rows[keyspace_name][table_name]
trigger_rows = trigger_rows or self.keyspace_table_trigger_rows[keyspace_name][table_name]

Choose a reason for hiding this comment

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

Did you remove self.keyspace_table_trigger_rows as well?

Comment on lines -2698 to -2701
for trigger_row in trigger_rows:
trigger_meta = self._build_trigger_metadata(table_meta, trigger_row)
table_meta.triggers[trigger_meta.name] = trigger_meta

Choose a reason for hiding this comment

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

And table_meta.triggers?

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.

2 participants