Claudenw commented on code in PR #3168:
URL: https://github.com/apache/cassandra/pull/3168#discussion_r1549749700
##########
src/java/org/apache/cassandra/schema/TableParams.java:
##########
@@ -490,6 +490,11 @@ public Builder extensions(Map<String, ByteBuffer> val)
public static class Serializer implements MetadataSerializer<TableParams>
{
+ private final String keyspace;
Review Comment:
If we leave the `CompressionParams.defaultParams(keyspace)` method in place
then everywhere we need to create compression params we pass the keyspace. and
99% of the code is correct.
The only place where this causes a problem is in the TableParam
serialization, as noted above. There are 2 cases to consider:
1. Serialization of TableParms to a stream. In this case the
CompressionParams are already set and we don't need to construct them.
2. Deserialization of TableParms from a stream. In this case the
CompressionParams should have been serialized. If they were serialized as an
empty map then they are the old style default, and we can get that directly
from the CompressionParams class.
I don't see why we need to do a partial build followed by a setting of the
params followed by the rest of the build. This sounds like a recipe for
disaster going forward. Let's keep it as simple as possible, but no simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
