yifan-c commented on code in PR #4411:
URL: https://github.com/apache/cassandra/pull/4411#discussion_r2516110226


##########
conf/cassandra.yaml:
##########
@@ -2592,7 +2571,15 @@ drop_compact_storage_enabled: false
 # method in GuardrailsMBean. It will effectively forbid the reconfiguration of 
password validator in runtime.
 # You would need to stop the node, change the configuration in cassandra.yaml 
and start the node again.
 # Defaults to true, which means that reconfiguration of password validator via 
JMX is possible.
-#password_policy_reconfiguration_enabled: true
+# password_validator_reconfiguration_enabled: true
+
+# Maximum allowed length for comments on schema elements (keyspaces, tables, 
columns, types, type fields).
+# Comments exceeding this length will be rejected. Defaults to 128 characters.
+max_comment_length: 128
+
+# Maximum allowed length for security labels on schema elements (tables, 
columns).
+# Security labels exceeding this length will be rejected. Defaults to 48 
characters.
+max_security_label_length: 48
 

Review Comment:
   👍 



##########
src/java/org/apache/cassandra/schema/Types.java:
##########
@@ -498,16 +572,95 @@ public long serializedSize(Types t, Version version)
             for (UserType type : t.types.values())
             {
                 size += sizeof(type.getNameAsString());
-                List<String> fieldNames = 
type.fieldNames().stream().map(FieldIdentifier::toString).collect(toList());
-                List<String> fieldTypes = 
type.fieldTypes().stream().map(AbstractType::asCQL3Type).map(CQL3Type::toString).collect(toList());
+                List<FieldIdentifier> fieldIdentifiers = type.fieldNames();
+                List<String> fieldNames = 
fieldIdentifiers.stream().map(FieldIdentifier::toString).collect(toList());
+                List<String> fieldTypes = 
type.fieldTypes().stream().map(AbstractType::asCQL3Type).map(CQL3Type::toString).collect(toList());;
                 size += sizeof(fieldNames.size());
                 for (String s : fieldNames)
                     size += sizeof(s);
                 size += sizeof(fieldTypes.size());
                 for (String s : fieldTypes)
                     size += sizeof(s);
+                if (version.isAtLeast(Version.V8))
+                {
+                    size += sizeof(type.comment);
+                    size += sizeof(type.securityLabel);
+                    size += fieldMetadataSize(type, fieldIdentifiers);
+                }
             }
             return size;
         }
+
+        private void serializeFieldMetadata(UserType type, 
List<FieldIdentifier> fieldIdentifiers, DataOutputPlus out) throws IOException
+        {
+            // Sparse serialization: only serialize non-empty metadata with 
their positions
+            serializeSparseMetadata(fieldIdentifiers, type::fieldComment, out);
+            serializeSparseMetadata(fieldIdentifiers, 
type::fieldSecurityLabel, out);
+        }
+
+        private void serializeSparseMetadata(List<FieldIdentifier> 
fieldIdentifiers,
+                                             Function<FieldIdentifier, String> 
metadataGetter,
+                                             DataOutputPlus out) throws 
IOException
+        {
+            // Collect only non-empty indexes
+            List<Integer> nonEmptyIndexes = new ArrayList<>();
+            for (int i = 0; i < fieldIdentifiers.size(); i++)
+            {
+                String value = metadataGetter.apply(fieldIdentifiers.get(i));
+                if (!value.isEmpty())
+                    nonEmptyIndexes.add(i);
+            }
+
+            // Write count followed by position-value pairs
+            out.writeUnsignedVInt32(nonEmptyIndexes.size());
+            for (int index : nonEmptyIndexes)
+            {
+                out.writeUnsignedVInt32(index);
+                
out.writeUTF(metadataGetter.apply(fieldIdentifiers.get(index)));
+            }
+        }
+
+        private void deserializeFieldMetadata(DataInputPlus in, List<String> 
fieldComments, List<String> fieldSecurityLabels) throws IOException
+        {
+            deserializeSparseMetadata(in, fieldComments);
+            deserializeSparseMetadata(in, fieldSecurityLabels);
+        }
+
+        private void deserializeSparseMetadata(DataInputPlus in, List<String> 
target) throws IOException
+        {
+            int count = in.readUnsignedVInt32();
+            for (int i = 0; i < count; i++)
+            {
+                int position = in.readUnsignedVInt32();
+                String value = in.readUTF();
+                target.set(position, value);
+            }
+        }
+
+        private long fieldMetadataSize(UserType type, List<FieldIdentifier> 
fieldIdentifiers)
+        {
+            return sparseMetadataSize(fieldIdentifiers, type::fieldComment)
+                 + sparseMetadataSize(fieldIdentifiers, 
type::fieldSecurityLabel);
+        }
+
+        private long sparseMetadataSize(List<FieldIdentifier> fieldIdentifiers,
+                                       
java.util.function.Function<FieldIdentifier, String> metadataGetter)
+        {
+            int nonEmptyCount = 0;
+            long dataSize = 0;
+
+            for (FieldIdentifier fieldId : fieldIdentifiers)
+            {
+                String value = metadataGetter.apply(fieldId);
+                if (!value.isEmpty())
+                {
+                    nonEmptyCount++;
+                    dataSize += sizeofUnsignedVInt(0);     // position (int)

Review Comment:
   You cannot get the size of 0 for all items. `sizeofUnsignedVInt` can return 
different values based on the input. 
   Please use the actual index value, instead of 0.



##########
src/java/org/apache/cassandra/db/marshal/UserType.java:
##########
@@ -442,14 +471,66 @@ public UserType withUpdatedUserType(UserType udt)
         {
             return isMultiCell == udt.isMultiCell
                  ? udt
-                 : new UserType(keyspace, name, udt.fieldNames(), 
udt.fieldTypes(), isMultiCell);
+                 : new UserType(keyspace, name, udt.fieldNames(), 
udt.fieldTypes(), isMultiCell, udt.comment, udt.securityLabel, 
udt.fieldComments, udt.fieldSecurityLabels);
         }
 
         return new UserType(keyspace,
                             name,
                             fieldNames,
                             Lists.newArrayList(transform(fieldTypes(), t -> 
t.withUpdatedUserType(udt))),
-                            isMultiCell());
+                            isMultiCell(),
+                            comment,
+                            securityLabel,
+                            fieldComments,
+                            fieldSecurityLabels);
+    }
+
+    public UserType withComment(String comment)
+    {
+        return new UserType(keyspace, name, fieldNames, fieldTypes(), 
isMultiCell(), comment, securityLabel, fieldComments, fieldSecurityLabels);
+    }
+
+    public UserType withSecurityLabel(String securityLabel)
+    {
+        return new UserType(keyspace, name, fieldNames, fieldTypes(), 
isMultiCell(), comment, securityLabel, fieldComments, fieldSecurityLabels);
+    }
+
+    public UserType withFieldComment(FieldIdentifier fieldName, String 
fieldComment)
+    {
+        if (fieldPosition(fieldName) == -1)
+            throw new IllegalArgumentException(String.format("Field '%s' 
doesn't exist in type '%s.%s'", fieldName, keyspace, getNameAsString()));
+
+        Map<FieldIdentifier, String> newFieldComments = new 
HashMap<>(fieldComments);
+        if (fieldComment == null || fieldComment.isEmpty())

Review Comment:
   Do we still allow empty string or the condition is just there to be safe? 
Please make a comment at least. 



-- 
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]

Reply via email to