You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by if...@apache.org on 2020/01/22 18:57:21 UTC

[cassandra] branch cassandra-3.0 updated: Make sure that hidden columns are only hidden from CQL

This is an automated email from the ASF dual-hosted git repository.

ifesdjeen pushed a commit to branch cassandra-3.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/cassandra-3.0 by this push:
     new b907dc9  Make sure that hidden columns are only hidden from CQL
b907dc9 is described below

commit b907dc9689dd04ebae1f765570401d1f20a88ebd
Author: Alex Petrov <ol...@gmail.com>
AuthorDate: Thu Jan 16 15:22:57 2020 +0100

    Make sure that hidden columns are only hidden from CQL
    
    Patch by Alex Petrov; reviewed by Aleksandr Sorokoumov for CASSANDRA-13917 (follow-up).
---
 .../org/apache/cassandra/config/CFMetaData.java    | 26 +++++++++++++---------
 .../apache/cassandra/cql3/ColumnIdentifier.java    |  2 +-
 src/java/org/apache/cassandra/cql3/Relation.java   |  2 +-
 .../cassandra/cql3/selection/Selectable.java       |  2 +-
 .../cql3/statements/CreateIndexStatement.java      |  2 +-
 .../cql3/statements/ModificationStatement.java     |  6 ++---
 .../cassandra/cql3/statements/SelectStatement.java |  7 +++---
 .../cassandra/db/AbstractReadCommandBuilder.java   |  3 +++
 .../org/apache/cassandra/db/RowUpdateBuilder.java  |  2 +-
 test/unit/org/apache/cassandra/cql3/ViewTest.java  | 19 ++++++++++++++++
 .../cql3/validation/operations/AlterTest.java      |  3 +++
 .../cql3/validation/operations/InsertTest.java     | 12 +++++-----
 .../cql3/validation/operations/SelectTest.java     |  4 ++++
 .../index/internal/CassandraIndexTest.java         |  9 ++++++++
 14 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index 5888f42..68e06be 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -782,11 +782,6 @@ public final class CFMetaData
         return compactValueColumn;
     }
 
-    private boolean isHiddenColumn(ColumnDefinition def)
-    {
-        return hiddenColumns.contains(def);
-    }
-
     public ClusteringComparator getKeyValidatorAsClusteringComparator()
     {
         boolean isCompound = keyValidator instanceof CompositeType;
@@ -979,7 +974,7 @@ public final class CFMetaData
      */
     public ColumnDefinition getColumnDefinition(ColumnIdentifier name)
     {
-       return getColumnDefinition(name.bytes);
+        return getColumnDefinition(name.bytes);
     }
 
     // In general it is preferable to work with ColumnIdentifier to make it
@@ -988,10 +983,19 @@ public final class CFMetaData
     // for instance) so...
     public ColumnDefinition getColumnDefinition(ByteBuffer name)
     {
-        ColumnDefinition cd = columnMetadata.get(name);
-        if (cd == null || isHiddenColumn(cd))
-            return null;
-        return cd;
+        return columnMetadata.get(name);
+    }
+
+    // Returns only columns that are supposed to be visible through CQL layer
+    public ColumnDefinition getColumnDefinitionForCQL(ColumnIdentifier name)
+    {
+        return getColumnDefinitionForCQL(name.bytes);
+    }
+
+    public ColumnDefinition getColumnDefinitionForCQL(ByteBuffer name)
+    {
+        ColumnDefinition cd = getColumnDefinition(name);
+        return hiddenColumns.contains(cd) ? null : cd;
     }
 
     public static boolean isNameValid(String name)
@@ -1130,7 +1134,7 @@ public final class CFMetaData
 
     public void renameColumn(ColumnIdentifier from, ColumnIdentifier to) throws InvalidRequestException
     {
-        ColumnDefinition def = getColumnDefinition(from);
+        ColumnDefinition def = getColumnDefinitionForCQL(from);
 
         if (def == null)
             throw new InvalidRequestException(String.format("Cannot rename unknown column %s in keyspace %s", from, cfName));
diff --git a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
index 71e7b9a..5d4e992 100644
--- a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
+++ b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java
@@ -222,7 +222,7 @@ public class ColumnIdentifier extends Selectable implements IMeasurableMemory, C
 
     public Selector.Factory newSelectorFactory(CFMetaData cfm, List<ColumnDefinition> defs) throws InvalidRequestException
     {
-        ColumnDefinition def = cfm.getColumnDefinition(this);
+        ColumnDefinition def = cfm.getColumnDefinitionForCQL(this);
         if (def == null)
             throw new InvalidRequestException(String.format("Undefined name %s in selection clause", this));
 
diff --git a/src/java/org/apache/cassandra/cql3/Relation.java b/src/java/org/apache/cassandra/cql3/Relation.java
index a88932e..005d984 100644
--- a/src/java/org/apache/cassandra/cql3/Relation.java
+++ b/src/java/org/apache/cassandra/cql3/Relation.java
@@ -263,7 +263,7 @@ public abstract class Relation {
                                                         ColumnIdentifier.Raw entity) throws InvalidRequestException
     {
         ColumnIdentifier identifier = entity.prepare(cfm);
-        ColumnDefinition def = cfm.getColumnDefinition(identifier);
+        ColumnDefinition def = cfm.getColumnDefinitionForCQL(identifier);
 
         if (def == null)
             throw new UnrecognizedEntityException(identifier, this);
diff --git a/src/java/org/apache/cassandra/cql3/selection/Selectable.java b/src/java/org/apache/cassandra/cql3/selection/Selectable.java
index 717fe7c..653a86a 100644
--- a/src/java/org/apache/cassandra/cql3/selection/Selectable.java
+++ b/src/java/org/apache/cassandra/cql3/selection/Selectable.java
@@ -77,7 +77,7 @@ public abstract class Selectable
         public Selector.Factory newSelectorFactory(CFMetaData cfm,
                                                    List<ColumnDefinition> defs) throws InvalidRequestException
         {
-            ColumnDefinition def = cfm.getColumnDefinition(id);
+            ColumnDefinition def = cfm.getColumnDefinitionForCQL(id);
             if (def == null)
                 throw new InvalidRequestException(String.format("Undefined name %s in selection clause", id));
             if (def.isPrimaryKeyColumn())
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
index 47d54fe..e0b9b02 100644
--- a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
@@ -97,7 +97,7 @@ public class CreateIndexStatement extends SchemaAlteringStatement
 
         for (IndexTarget target : targets)
         {
-            ColumnDefinition cd = cfm.getColumnDefinition(target.column);
+            ColumnDefinition cd = cfm.getColumnDefinitionForCQL(target.column);
 
             if (cd == null)
                 throw new InvalidRequestException("No column definition found for column " + target.column);
diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
index f4ad8b2..65fa948 100644
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@ -862,8 +862,8 @@ public abstract class ModificationStatement implements CQLStatement
             for (Pair<ColumnIdentifier.Raw, ColumnCondition.Raw> entry : conditions)
             {
                 ColumnIdentifier id = entry.left.prepare(metadata);
-                ColumnDefinition def = metadata.getColumnDefinition(id);
-                checkNotNull(metadata.getColumnDefinition(id), "Unknown identifier %s in IF conditions", id);
+                ColumnDefinition def = metadata.getColumnDefinitionForCQL(id);
+                checkNotNull(def, "Unknown identifier %s in IF conditions", id);
 
                 ColumnCondition condition = entry.right.prepare(keyspace(), def);
                 condition.collectMarkerSpecification(boundNames);
@@ -912,7 +912,7 @@ public abstract class ModificationStatement implements CQLStatement
         protected static ColumnDefinition getColumnDefinition(CFMetaData cfm, Raw rawId)
         {
             ColumnIdentifier id = rawId.prepare(cfm);
-            return checkNotNull(cfm.getColumnDefinition(id), "Unknown identifier %s", id);
+            return checkNotNull(cfm.getColumnDefinitionForCQL(id), "Unknown identifier %s", id);
         }
     }
 }
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index a5e6254..30c4458 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -1031,7 +1030,7 @@ public class SelectStatement implements CQLStatement
             for (ColumnIdentifier.Raw raw : parameters.orderings.keySet())
             {
                 ColumnIdentifier identifier = raw.prepare(cfm);
-                ColumnDefinition orderingColumn = cfm.getColumnDefinition(identifier);
+                ColumnDefinition orderingColumn = cfm.getColumnDefinitionForCQL(identifier);
                 idToSort.add(orderingIndexes.get(orderingColumn));
                 sorters.add(orderingColumn.type);
             }
@@ -1048,7 +1047,7 @@ public class SelectStatement implements CQLStatement
             for (ColumnIdentifier.Raw raw : parameters.orderings.keySet())
             {
                 ColumnIdentifier column = raw.prepare(cfm);
-                final ColumnDefinition def = cfm.getColumnDefinition(column);
+                final ColumnDefinition def = cfm.getColumnDefinitionForCQL(column);
                 if (def == null)
                     handleUnrecognizedOrderingColumn(column);
                 selection.addColumnForOrdering(def);
@@ -1065,7 +1064,7 @@ public class SelectStatement implements CQLStatement
                 ColumnIdentifier column = entry.getKey().prepare(cfm);
                 boolean reversed = entry.getValue();
 
-                ColumnDefinition def = cfm.getColumnDefinition(column);
+                ColumnDefinition def = cfm.getColumnDefinitionForCQL(column);
                 if (def == null)
                     handleUnrecognizedOrderingColumn(column);
 
diff --git a/src/java/org/apache/cassandra/db/AbstractReadCommandBuilder.java b/src/java/org/apache/cassandra/db/AbstractReadCommandBuilder.java
index d219816..a7f3319 100644
--- a/src/java/org/apache/cassandra/db/AbstractReadCommandBuilder.java
+++ b/src/java/org/apache/cassandra/db/AbstractReadCommandBuilder.java
@@ -21,6 +21,8 @@ package org.apache.cassandra.db;
 import java.nio.ByteBuffer;
 import java.util.*;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.ColumnDefinition;
 import org.apache.cassandra.cql3.ColumnIdentifier;
@@ -161,6 +163,7 @@ public abstract class AbstractReadCommandBuilder
         throw new AssertionError();
     }
 
+    @VisibleForTesting
     public AbstractReadCommandBuilder filterOn(String column, Operator op, Object value)
     {
         ColumnDefinition def = cfs.metadata.getColumnDefinition(ColumnIdentifier.getInterned(column, true));
diff --git a/src/java/org/apache/cassandra/db/RowUpdateBuilder.java b/src/java/org/apache/cassandra/db/RowUpdateBuilder.java
index 8ace988..c4b4c75 100644
--- a/src/java/org/apache/cassandra/db/RowUpdateBuilder.java
+++ b/src/java/org/apache/cassandra/db/RowUpdateBuilder.java
@@ -392,7 +392,7 @@ public class RowUpdateBuilder
 
     private ColumnDefinition getDefinition(String name)
     {
-        return update.metadata().getColumnDefinition(new ColumnIdentifier(name, true));
+        return update.metadata().getColumnDefinitionForCQL(new ColumnIdentifier(name, true));
     }
 
     public UnfilteredRowIterator unfilteredIterator()
diff --git a/test/unit/org/apache/cassandra/cql3/ViewTest.java b/test/unit/org/apache/cassandra/cql3/ViewTest.java
index 136ae1c..0d49e4b 100644
--- a/test/unit/org/apache/cassandra/cql3/ViewTest.java
+++ b/test/unit/org/apache/cassandra/cql3/ViewTest.java
@@ -1389,4 +1389,23 @@ public class ViewTest extends CQLTester
      {
          execute("CREATE MATERIALIZED VIEW myview AS SELECT a, b FROM \"\" WHERE b IS NOT NULL PRIMARY KEY (b, a)");
      }
+
+    @Test
+    public void viewOnCompactTableTest() throws Throwable
+    {
+        createTable("CREATE TABLE %s (a int, b int, v int, PRIMARY KEY (a, b)) WITH COMPACT STORAGE");
+        executeNet(protocolVersion, "USE " + keyspace());
+        try
+        {
+            createView("mv",
+                       "CREATE MATERIALIZED VIEW %s AS SELECT a, b, value FROM %%s WHERE b IS NOT NULL PRIMARY KEY (b, a)");
+            fail("Should have thrown an exception");
+        }
+        catch (Throwable t)
+        {
+            Assert.assertEquals("Unknown column name detected in CREATE MATERIALIZED VIEW statement : value",
+                                t.getMessage());
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
index da27dac..d97583a 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
@@ -487,6 +487,9 @@ public class AlterTest extends CQLTester
 
         assertInvalidMessage("Cannot rename unknown column column1 in keyspace",
                              "ALTER TABLE %s RENAME column1 TO column2");
+
+        assertInvalidMessage("Cannot rename unknown column value in keyspace",
+                             "ALTER TABLE %s RENAME value TO value2");
     }
 
     /**
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertTest.java
index 8c42668..e467291 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/InsertTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/InsertTest.java
@@ -234,24 +234,24 @@ public class InsertTest extends CQLTester
         // pass correct types to the hidden columns
         assertInvalidMessage("Unknown identifier column1",
                              "INSERT INTO %s (a, b, column1) VALUES (?, ?, ?)",
-                             1, 1, 1, ByteBufferUtil.bytes('a'));
+                             1, 1, ByteBufferUtil.bytes('a'));
         assertInvalidMessage("Unknown identifier value",
                              "INSERT INTO %s (a, b, value) VALUES (?, ?, ?)",
-                             1, 1, 1, ByteBufferUtil.bytes('a'));
+                             1, 1, ByteBufferUtil.bytes('a'));
         assertInvalidMessage("Unknown identifier column1",
                              "INSERT INTO %s (a, b, column1, value) VALUES (?, ?, ?, ?)",
-                             1, 1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
+                             1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
         assertInvalidMessage("Unknown identifier value",
                              "INSERT INTO %s (a, b, value, column1) VALUES (?, ?, ?, ?)",
-                             1, 1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
+                             1, 1, ByteBufferUtil.bytes('a'), ByteBufferUtil.bytes('b'));
 
         // pass incorrect types to the hidden columns
         assertInvalidMessage("Unknown identifier value",
                              "INSERT INTO %s (a, b, value) VALUES (?, ?, ?)",
-                             1, 1, 1, 1);
+                             1, 1, 1);
         assertInvalidMessage("Unknown identifier column1",
                              "INSERT INTO %s (a, b, column1) VALUES (?, ?, ?)",
-                             1, 1, 1, 1);
+                             1, 1, 1);
         assertEmpty(execute("SELECT * FROM %s"));
 
         // pass null to the hidden columns
diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
index 6182f11..f88dd2f 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/SelectTest.java
@@ -3083,6 +3083,10 @@ public class SelectTest extends CQLTester
 
     private void testWithCompactFormat() throws Throwable
     {
+        assertInvalidMessage("Order by on unknown column value",
+                             "SELECT * FROM %s WHERE a IN (1,2,3) ORDER BY value ASC");
+        assertInvalidMessage("Order by on unknown column column1",
+                             "SELECT * FROM %s WHERE a IN (1,2,3) ORDER BY column1 ASC");
         assertInvalidMessage("Undefined name column1 in selection clause",
                              "SELECT column1 FROM %s");
         assertInvalidMessage("Undefined name value in selection clause",
diff --git a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
index 68fe42d..286c418 100644
--- a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
+++ b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java
@@ -42,6 +42,7 @@ import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.utils.ByteBufferUtil;
 import org.apache.cassandra.utils.FBUtilities;
 
+import static org.apache.cassandra.Util.executeLocally;
 import static org.apache.cassandra.Util.throwAssert;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -339,6 +340,14 @@ public class CassandraIndexTest extends CQLTester
     }
 
     @Test
+    public void testIndexOnCompactTable() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int, v int, PRIMARY KEY (k)) WITH COMPACT STORAGE;");
+        assertInvalidMessage("No column definition found for column value",
+                             "CREATE INDEX idx_value ON %s(value)");
+    }
+
+    @Test
     public void indexOnClusteringColumnWithoutRegularColumns() throws Throwable
     {
         Object[] row1 = row("k0", "c0");


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