Skip to content

Expose settings in virtual table for CASSANDRA-14573 - #242

Closed
clohfink wants to merge 5 commits into
apache:trunkfrom
clohfink:trunk-14573
Closed

Expose settings in virtual table for CASSANDRA-14573#242
clohfink wants to merge 5 commits into
apache:trunkfrom
clohfink:trunk-14573

Conversation

@clohfink

Copy link
Copy Markdown
Contributor

No description provided.

@clohfink
clohfink force-pushed the trunk-14573 branch 2 times, most recently from ade798f to a4463e7 Compare August 14, 2018 15:56

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.

Not that I mind the change, but it isn't necessary for this patch, and there is metadata() if it were.

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.

A lot of unused imports here.

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.

Missing ASF license.

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.

Doesn't really need to be static. Can also be a map of name -> Field, so that data(DecoratedKey partitionKey) is cleaner.

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.

Doesn't handle array values like the other data() implementation.

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.

Should probably factor out this whole block into a method usable by both data() implementations. String fieldValue(Field field, Config config)?

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.

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.

Also import order.

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.

Is this the toString() implementation we want? Like the other three, it's missing fields, intentionally or not (if intentionally, maybe comment why so?). Also, this string helper prefixes the map with the class name, and it doesn't look pretty in CQL output, subjectively.

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.

It's not invalid to query a table for a non-existent partition key, you just return an empty data set in CQL.

blambov pushed a commit to blambov/cassandra that referenced this pull request Sep 10, 2021
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.
blambov pushed a commit to blambov/cassandra that referenced this pull request Mar 21, 2022
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
blambov pushed a commit to blambov/cassandra that referenced this pull request Jun 13, 2022
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
blambov pushed a commit to blambov/cassandra that referenced this pull request Nov 24, 2022
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
(cherry picked from commit 1b2034e)
adelapena pushed a commit to adelapena/cassandra that referenced this pull request Sep 26, 2023
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
(cherry picked from commit 1b2034e)
(cherry picked from commit b24d83f)
(cherry picked from commit 19e5337)
ekaterinadimitrova2 pushed a commit to ekaterinadimitrova2/cassandra that referenced this pull request Jun 3, 2024
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
(cherry picked from commit 1b2034e)
(cherry picked from commit b24d83f)
(cherry picked from commit 19e5337)
(cherry picked from commit a790c81)

STAR-789 Fix order of CUSTOM_TMD_PROVIDER_PROPERTY in CassandraRelevantProperties
michaelsembwever pushed a commit to thelastpickle/cassandra that referenced this pull request Jan 7, 2026
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
(cherry picked from commit 1b2034e)
(cherry picked from commit b24d83f)
(cherry picked from commit 19e5337)
(cherry picked from commit a790c81)

STAR-789 Fix order of CUSTOM_TMD_PROVIDER_PROPERTY in CassandraRelevantProperties
lesnik2u pushed a commit to lesnik2u/cassandra that referenced this pull request May 26, 2026
…oken metadata) (apache#242)

Introduced TokenMetadataProvider to abstract access to TokenMetadata and make it pluggable.

(cherry picked from commit 8c0a970)
(cherry picked from commit 58b15c2)
(cherry picked from commit 1b2034e)
(cherry picked from commit b24d83f)
(cherry picked from commit 19e5337)
(cherry picked from commit a790c81)

STAR-789 Fix order of CUSTOM_TMD_PROVIDER_PROPERTY in CassandraRelevantProperties

 (Rebase of commit c698031)
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.

3 participants