Skip to content
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.

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

Better printing of CDC options in DESCRIBE TABLE in cqlsh #245

Open
avelanarius opened this issue Apr 14, 2021 · 7 comments
Open

Better printing of CDC options in DESCRIBE TABLE in cqlsh #245

avelanarius opened this issue Apr 14, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@avelanarius
Copy link

Currently, cqlsh prints CDC options in DESCRIBE TABLE queries in a separate line:

cqlsh> DESCRIBE TABLE ks.t;

CREATE TABLE ks.t (
    pk int,
    ck int,
    v int,
    v2 int,
    PRIMARY KEY (pk, ck)
) WITH CLUSTERING ORDER BY (ck ASC)
    AND bloom_filter_fp_chance = 0.01
    AND caching = {'keys': 'ALL', 'rows_per_partition': 'ALL'}
    AND comment = ''
    AND compaction = {'class': 'SizeTieredCompactionStrategy'}
    AND compression = {'sstable_compression': 'org.apache.cassandra.io.compress.LZ4Compressor'}
    AND crc_check_chance = 1.0
    AND dclocal_read_repair_chance = 0.0
    AND default_time_to_live = 0
    AND gc_grace_seconds = 864000
    AND max_index_interval = 2048
    AND memtable_flush_period_in_ms = 0
    AND min_index_interval = 128
    AND read_repair_chance = 0.0
    AND speculative_retry = '99.0PERCENTILE';

cdc = {'postimage': 'false', 'preimage': 'true', 'ttl': '86400', 'enabled': 'true', 'delta': 'full'}

I think it would be better to print those options not in a separate line, but within the CREATE TABLE statement. That way it is more consistent with other options and easily copyable:

cqlsh> DESCRIBE TABLE ks.t;

CREATE TABLE ks.t (
    pk int,
    ck int,
    v int,
    v2 int,
    PRIMARY KEY (pk, ck)
) WITH CLUSTERING ORDER BY (ck ASC)
    AND bloom_filter_fp_chance = 0.01
    AND caching = {'keys': 'ALL', 'rows_per_partition': 'ALL'}
    AND comment = ''
    AND compaction = {'class': 'SizeTieredCompactionStrategy'}
    AND compression = {'sstable_compression': 'org.apache.cassandra.io.compress.LZ4Compressor'}
    AND cdc = {'postimage': 'false', 'preimage': 'true', 'ttl': '86400', 'enabled': 'true', 'delta': 'full'}
    AND crc_check_chance = 1.0
    AND dclocal_read_repair_chance = 0.0
    AND default_time_to_live = 0
    AND gc_grace_seconds = 864000
    AND max_index_interval = 2048
    AND memtable_flush_period_in_ms = 0
    AND min_index_interval = 128
    AND read_repair_chance = 0.0
    AND speculative_retry = '99.0PERCENTILE';

(for a reference, Java Driver 3.x already prints it in that format when using toString() after this commit: scylladb/java-driver@ae2254a)

@avelanarius avelanarius added the enhancement New feature or request label Apr 14, 2021
@avelanarius
Copy link
Author

/cc @jul-stas (I think you implemented printing of CDC options in cqlsh, was there a important reason for printing those options in a separate line?)

@jul-stas
Copy link
Contributor

@avelanarius IIRC the reason was to separate schema extensions from regular table options. In general, putting schema extensions among "normal" options, like AND schema_extension_X = abc, can lead to an incorrect CREATE statement.
Maybe it's not the case with CDC schema extension so it can have special treatment while printing...

@fruch
Copy link
Contributor

fruch commented Jul 30, 2023

I think server side DESCRIBE should solve this one...

@avelanarius
Copy link
Author

Small note: CDC options in server-side DESCRIBE were added only recently (5.2, 5.3 doesn't have it): scylladb/scylladb@62ced66

@mykaul
Copy link
Contributor

mykaul commented Dec 28, 2023

@fruch - shall I move it to cqlsh repo?

@fruch
Copy link
Contributor

fruch commented Dec 28, 2023

@fruch - shall I move it to cqlsh repo?

You can move it

But it would need to be part of the scylla core as well, cause the server side describe

@mykaul
Copy link
Contributor

mykaul commented Jan 1, 2024

@fruch - shall I move it to cqlsh repo?

You can move it

But it would need to be part of the scylla core as well, cause the server side describe

I'll need to understand better what is needed in core - please open a separate issue if neede.

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

No branches or pull requests

4 participants