scylladb / scylla-tools-java

Apache Cassandra, supplying tools for Scylla
Apache License 2.0
52 stars 81 forks source link

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

Open avelanarius opened 3 years ago

avelanarius commented 3 years ago

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: https://github.com/scylladb/java-driver/commit/ae2254a2e6458fe99bb764af24d68df2590171d7)

avelanarius commented 3 years ago

/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 commented 3 years ago

@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 commented 11 months ago

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

avelanarius commented 11 months ago

Small note: CDC options in server-side DESCRIBE were added only recently (5.2, 5.3 doesn't have it): https://github.com/scylladb/scylladb/commit/62ced6670231aa013acdfb2d1f4913a7b238e2db

mykaul commented 6 months ago

@fruch - shall I move it to cqlsh repo?

fruch commented 6 months ago

@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 commented 6 months ago

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