You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/07/22 18:26:25 UTC

[GitHub] [cassandra] yifan-c commented on a diff in pull request #1739: CASSANDRA-8877 trunk: Ability to read the TTL and WRITE TIME of an element in a collection

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


##########
NEWS.txt:
##########
@@ -63,9 +63,11 @@ New features
       If this is set to false, streaming will be considerably faster however it's possible that, in extreme situations
       (losing > quorum # nodes in a replica set), you may have data in your SSTables that never makes it to the CDC log.
       The default is true/enabled. The configuration can be altered via JMX.
+    - Added support for reading the write times and TTLs of the elements of collections and UDTs, frozen or not. The CQL

Review Comment:
   nit: "frozen or not" --> "regardless being frozen or not"



##########
NEWS.txt:
##########
@@ -63,9 +63,11 @@ New features
       If this is set to false, streaming will be considerably faster however it's possible that, in extreme situations
       (losing > quorum # nodes in a replica set), you may have data in your SSTables that never makes it to the CDC log.
       The default is true/enabled. The configuration can be altered via JMX.
+    - Added support for reading the write times and TTLs of the elements of collections and UDTs, frozen or not. The CQL
+      functions writetime, maxwritetime and ttl can now be applied to entire collections/UDTs, single collection/UDT
+      elements and slices of collection/UDT elements.
     - Added a new CQL function, maxwritetime. It shows the largest unix timestamp that the data was written, similar to
-      its sibling CQL function, writetime. Unlike writetime, maxwritetime can be applied to multi-cell data types, e.g.
-      non-frozen collections and UDT, and returns the largest timestamp. One should not to use it when upgrading to 4.2.
+      its sibling CQL function, writetime. One should not to use it when upgrading to 4.2.

Review Comment:
   This one is my bad.
   `One should not to use it` --> `One should not use it`



##########
src/antlr/Parser.g:
##########
@@ -414,12 +414,12 @@ simpleUnaliasedSelector returns [Selectable.Raw s]
     ;
 
 selectionFunction returns [Selectable.Raw s]
-    : K_COUNT '(' '\*' ')'                      { $s = Selectable.WithFunction.Raw.newCountRowsFunction(); }
-    | K_MAXWRITETIME '(' c=sident ')'           { $s = new Selectable.WritetimeOrTTL.Raw(c, Selectable.WritetimeOrTTL.Kind.MAX_WRITE_TIME); }
-    | K_WRITETIME '(' c=sident ')'              { $s = new Selectable.WritetimeOrTTL.Raw(c, Selectable.WritetimeOrTTL.Kind.WRITE_TIME); }
-    | K_TTL       '(' c=sident ')'              { $s = new Selectable.WritetimeOrTTL.Raw(c, Selectable.WritetimeOrTTL.Kind.TTL); }
-    | K_CAST      '(' sn=unaliasedSelector K_AS t=native_type ')' {$s = new Selectable.WithCast.Raw(sn, t);}
-    | f=functionName args=selectionFunctionArgs { $s = new Selectable.WithFunction.Raw(f, args); }
+    : K_COUNT        '(' '\*' ')'                                    { $s = Selectable.WithFunction.Raw.newCountRowsFunction(); }
+    | K_MAXWRITETIME '(' c=sident m=selectorModifier[c] ')'          { $s = new Selectable.WritetimeOrTTL.Raw(c, m, Selectable.WritetimeOrTTL.Kind.MAX_WRITE_TIME); }

Review Comment:
   now `maxwritetime` can be applied to a slice?



##########
src/java/org/apache/cassandra/serializers/SetSerializer.java:
##########
@@ -248,4 +253,88 @@ public ByteBuffer getSliceFromSerialized(ByteBuffer collection,
             throw new MarshalException("Not enough bytes to read a set");
         }
     }
+
+    @Override
+    public int getIndexFromSerialized(ByteBuffer collection, ByteBuffer key, AbstractType<?> comparator)
+    {
+        try
+        {
+            ByteBuffer input = collection.duplicate();
+            int n = readCollectionSize(input, ByteBufferAccessor.instance, ProtocolVersion.V3);
+            int offset = sizeOfCollectionSize(n, ProtocolVersion.V3);
+            for (int i = 0; i < n; i++)
+            {
+                ByteBuffer value = readValue(input, ByteBufferAccessor.instance, offset, ProtocolVersion.V3);
+                offset += sizeOfValue(value, ByteBufferAccessor.instance, ProtocolVersion.V3);
+                int comparison = comparator.compareForCQL(value, key);
+                if (comparison == 0)
+                    return i;
+                else if (comparison > 0)
+                    // since the set is in sorted order, we know we've gone too far and the element doesn't exist
+                    return -1;
+                // else, we're before the element so continue

Review Comment:
   nit: add curly brackets. I found it a bit difficult to read when being surrounded by comments. 



##########
src/java/org/apache/cassandra/cql3/selection/ColumnTimestamps.java:
##########
@@ -0,0 +1,381 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.cql3.selection;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import com.google.common.collect.BoundType;
+import com.google.common.collect.Range;
+
+import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.UserType;
+import org.apache.cassandra.db.rows.Cell;
+import org.apache.cassandra.serializers.CollectionSerializer;
+import org.apache.cassandra.transport.ProtocolVersion;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+/**
+ * Represents a list of timestamps associated to a CQL column. Those timestamps can either be writetimes or TTLs,
+ * according to {@link TimestampsType}.
+ */
+abstract class ColumnTimestamps
+{
+    /**
+     * The timestamps type.
+     */
+    protected final TimestampsType type;
+
+    protected ColumnTimestamps(TimestampsType type)
+    {
+        this.type = type;
+    }
+
+    /**
+     * @return the timestamps type
+     */
+    public TimestampsType type()
+    {
+        return type;
+    }
+
+    /**
+     * Retrieves the timestamps at the specified position.
+     *
+     * @param index the timestamps position
+     * @return the timestamps at the specified position or a {@link #NO_TIMESTAMP}
+     */
+    public abstract ColumnTimestamps get(int index);
+
+    public abstract ColumnTimestamps max();
+
+    /**
+     * Returns a view of the portion of the timestamps within the specified range.
+     *
+     * @param range the indexes range
+     * @return a view of the specified range within this {@link ColumnTimestamps}
+     */
+    public abstract ColumnTimestamps slice(Range<Integer> range);
+
+    /**
+     * Converts the timestamps into their serialized form.
+     *
+     * @param protocolVersion the protocol version to use for the serialization
+     * @return the serialized timestamps
+     */
+    public abstract ByteBuffer toByteBuffer(ProtocolVersion protocolVersion);
+
+    /**
+     * Appends an empty timestamp at the end of this list.
+     */
+    public abstract void addNoTimestamp();
+
+    /**
+     * Appends the timestamp of the specified cell at the end of this list.
+     */
+    public abstract void addTimestampFrom(Cell<?> cell, int nowInSecond);
+
+    /**
+     * Creates a new {@link ColumnTimestamps} instance for the specified column type.
+     *
+     * @param timestampType the timestamps type
+     * @param columnType    the column type
+     * @return a {@link ColumnTimestamps} instance for the specified column type
+     */
+    static ColumnTimestamps newTimestamps(TimestampsType timestampType, AbstractType<?> columnType)
+    {
+        if (!columnType.isMultiCell())
+            return new SingleTimestamps(timestampType);
+
+        // For UserType we know that the size will not change, so we can initialize the array with the proper capacity.
+        if (columnType instanceof UserType)
+            return new MultipleTimestamps(timestampType, ((UserType) columnType).size());
+
+        return new MultipleTimestamps(timestampType, 0);
+    }
+
+    /**
+     * The type of represented timestamps.
+     */
+    public enum TimestampsType
+    {
+        WRITETIMES
+        {
+            @Override
+            long getTimestamp(Cell<?> cell, int nowInSecond)
+            {
+                return cell.timestamp();
+            }
+
+            @Override
+            long defaultValue()
+            {
+                return Long.MIN_VALUE;
+            }
+
+            @Override
+            ByteBuffer toByteBuffer(long timestamp)
+            {
+                return timestamp == defaultValue() ? null : ByteBufferUtil.bytes(timestamp);
+            }
+        },
+        TTLS
+        {
+            @Override
+            long getTimestamp(Cell<?> cell, int nowInSecond)
+            {
+                if (!cell.isExpiring())
+                    return defaultValue();
+
+                int remaining = cell.localDeletionTime() - nowInSecond;
+                return remaining >= 0 ? remaining : defaultValue();
+            }
+
+            @Override
+            long defaultValue()
+            {
+                return -1;
+            }
+
+            @Override
+            ByteBuffer toByteBuffer(long timestamp)
+            {
+                return timestamp == defaultValue() ? null : ByteBufferUtil.bytes((int) timestamp);
+            }
+        };
+
+        /**
+         * Extracts the timestamp from the specified cell.
+         *
+         * @param cell        the cell
+         * @param nowInSecond the query timestamp insecond
+         * @return the timestamp corresponding to this type
+         */
+        abstract long getTimestamp(Cell<?> cell, int nowInSecond);
+
+        /**
+         * Returns the value to use when there is no timestamp.
+         *
+         * @return the value to use when there is no timestamp
+         */
+        abstract long defaultValue();
+
+        /**
+         * Serializes the specified timestamp.
+         *
+         * @param timestamp the timestamp to serialize
+         * @return the bytes corresponding to the specified timestamp
+         */
+        abstract ByteBuffer toByteBuffer(long timestamp);
+    }
+
+    /**
+     * A {@link ColumnTimestamps} that doesn't contain any timestamps.
+     */
+    static ColumnTimestamps NO_TIMESTAMP = new ColumnTimestamps(null)

Review Comment:
   Should it be `final`?



##########
src/java/org/apache/cassandra/cql3/selection/RowTimestamps.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.cql3.selection;
+
+import java.util.List;
+
+import org.apache.cassandra.db.rows.Cell;
+import org.apache.cassandra.schema.ColumnMetadata;
+
+/**
+ * The {@link ColumnTimestamps} associated to the given set of columns of a row.
+ */
+interface RowTimestamps
+{
+    /**
+     * Adds an empty timestamp for the specified column.
+     *
+     * @param index the column index
+     */
+    void addNoTimestamp(int index);
+
+    /**
+     * Adds the timestamp of the specified cell.
+     *
+     * @param index the column index
+     * @param cell the cell to get the timestamp from
+     * @param nowInSec the query timestamp in second
+     */
+    void addTimestamp(int index, Cell<?> cell, int nowInSec);
+
+    /**
+     * Returns the timestamp of the specified column.
+     *
+     * @param index the column index
+     * @return the timestamp of the specified column
+     */
+    ColumnTimestamps get(int index);
+
+    /**
+     * A {@code RowTimestamps} that does nothing.
+     */
+    static RowTimestamps NOOP_ROW_TIMESTAMPS = new RowTimestamps()

Review Comment:
   `static` is redundant since the variable is defined in an interface. 



##########
src/java/org/apache/cassandra/cql3/selection/WritetimeOrTTLSelector.java:
##########
@@ -32,51 +32,57 @@
 import org.apache.cassandra.io.util.DataInputPlus;
 import org.apache.cassandra.io.util.DataOutputPlus;
 import org.apache.cassandra.transport.ProtocolVersion;
-import org.apache.cassandra.utils.ByteBufferUtil;
 
 final class WritetimeOrTTLSelector extends Selector
 {
-    protected static final SelectorDeserializer deserializer = new SelectorDeserializer()
+    static final SelectorDeserializer deserializer = new SelectorDeserializer()
     {
+        @Override
         protected Selector deserialize(DataInputPlus in, int version, TableMetadata metadata) throws IOException
         {
-            ByteBuffer columnName = ByteBufferUtil.readWithVIntLength(in);
-            ColumnMetadata column = metadata.getColumn(columnName);
+            Selector selected = serializer.deserialize(in, version, metadata);
             int idx = in.readInt();
             int ordinal = in.readByte();
-            Selectable.WritetimeOrTTL.Kind k = Selectable.WritetimeOrTTL.Kind.fromOrdinal(ordinal);
-            return new WritetimeOrTTLSelector(column, idx, k);
+            Selectable.WritetimeOrTTL.Kind kind = Selectable.WritetimeOrTTL.Kind.fromOrdinal(ordinal);
+            boolean isMultiCell = in.readBoolean();
+            return new WritetimeOrTTLSelector(selected, idx, kind, isMultiCell);
         }
     };
 
-    private final ColumnMetadata column;
-    private final int idx;
+    private final Selector selected;
+    private final int columnIndex;
     private final Selectable.WritetimeOrTTL.Kind kind;
     private ByteBuffer current;
+    private final boolean isMultiCell;
     private boolean isSet;
 
-    public static Factory newFactory(final ColumnMetadata def, final int idx, final Selectable.WritetimeOrTTL.Kind kind)
+    public static Factory newFactory(final Selector.Factory factory, final int columnIndex, final Selectable.WritetimeOrTTL.Kind kind, boolean isMultiCell)
     {
         return new Factory()
         {
+            @Override
             protected String getColumnName()
             {
-                return String.format("%s(%s)", kind.name, def.name.toString());
+                return String.format("%s(%s)", kind.name, factory.getColumnName());
             }
 
+            @Override
             protected AbstractType<?> getReturnType()
             {
-                return kind.returnType;
+                AbstractType<?> type = kind.returnType;
+                return isMultiCell && !kind.aggregatesMultiCell() ? ListType.getInstance(type, false) : type;
             }
 
+            @Override
             protected void addColumnMapping(SelectionColumnMapping mapping, ColumnSpecification resultsColumn)
             {
-               mapping.addMapping(resultsColumn, def);
+                factory.addColumnMapping(mapping, resultsColumn);

Review Comment:
   nit: remove the extra heading space.



-- 
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: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org