Repository: gocql/gocql
Pull request #1436 (2020-04-29 19:33:47)

fix metadata views, incorrect query
Fixes https://github.com/gocql/gocql/issues/1435

jorgebay: I didn't have time to check it earlier, I'll review this on Monday.
jorgebay: Should we be concerned about breaking existing usage for `ViewMetadata` (udts)?

We have the option of leaving `ViewMetadata` as is and introduce a `MaterializedViewMetadata` for `system_schema.views`, wdyt?
beltran: Hmm, I may not fully understand what you mean, the problem I think it's that a bug that caused a change in the API was introduced. So you are suggesting adding below [here](https://github.com/gocql/gocql/blob/master/metadata.go#L25) a new line `MaterializedViews MaterializedViewMetadata` and leaving the data in `Views` as it is? Couldn't users get confused when they see both fields?
jorgebay: Yeah, I understand that it may cause confusion to new users (we can document the behaviour.  Introducing a new map+struct and leave the existing functionality as it is will avoid breaking usages of views as udts.
beltran: @jorgebay thank you for the feedback! I think this is ready for another looks. There's a failing test in one of the runs, but seems unrelated:
```
--- FAIL: TestPolicyConnPoolSSL (0.12s)
    conn_test.go:1191: write tcp 127.0.0.1:34668->127.0.0.1:34356: use of closed network connection
```

Patch:
diff --git a/metadata.go b/metadata.go
index 5a17559..6e4e0eb 100644
--- a/metadata.go
+++ b/metadata.go
@@ -22,7 +22,9 @@ type KeyspaceMetadata struct {
 	Tables          map[string]*TableMetadata
 	Functions       map[string]*FunctionMetadata
 	Aggregates      map[string]*AggregateMetadata
-	Views           map[string]*ViewMetadata
+	// Deprecated: use the MaterializedViews field instead.
+	Views             map[string]*ViewMetadata
+	MaterializedViews map[string]*MaterializedViewMetadata
 }
 
 // schema metadata for a table (a.k.a. column family)
@@ -83,6 +85,7 @@ type AggregateMetadata struct {
 }
 
 // ViewMetadata holds the metadata for views.
+// Deprecated: this is kept for backwards compatibility issues. Use MaterializedViewMetadata.
 type ViewMetadata struct {
 	Keyspace   string
 	Name       string
@@ -90,6 +93,33 @@ type ViewMetadata struct {
 	FieldTypes []TypeInfo
 }
 
+// MaterializedViewMetadata holds the metadata for materialized views.
+type MaterializedViewMetadata struct {
+	Keyspace                string
+	Name                    string
+	BaseTableId             UUID
+	BaseTable               *TableMetadata
+	BloomFilterFpChance     float64
+	Caching                 map[string]string
+	Comment                 string
+	Compaction              map[string]string
+	Compression             map[string]string
+	CrcCheckChance          float64
+	DcLocalReadRepairChance float64
+	DefaultTimeToLive       int
+	Extensions              map[string]string
+	GcGraceSeconds          int
+	Id                      UUID
+	IncludeAllColumns       bool
+	MaxIndexInterval        int
+	MemtableFlushPeriodInMs int
+	MinIndexInterval        int
+	ReadRepairChance        float64
+	SpeculativeRetry        string
+
+	baseTableName string
+}
+
 // the ordering of the column with regard to its comparator
 type ColumnOrder bool
 
@@ -246,9 +276,13 @@ func (s *schemaDescriber) refreshSchema(keyspaceName string) error {
 	if err != nil {
 		return err
 	}
+	materializedViews, err := getMaterializedViewsMetadata(s.session, keyspaceName)
+	if err != nil {
+		return err
+	}
 
 	// organize the schema data
-	compileMetadata(s.session.cfg.ProtoVersion, keyspace, tables, columns, functions, aggregates, views)
+	compileMetadata(s.session.cfg.ProtoVersion, keyspace, tables, columns, functions, aggregates, views, materializedViews)
 
 	// update the cache
 	s.cache[keyspaceName] = keyspace
@@ -269,6 +303,7 @@ func compileMetadata(
 	functions []FunctionMetadata,
 	aggregates []AggregateMetadata,
 	views []ViewMetadata,
+	materializedViews []MaterializedViewMetadata,
 ) {
 	keyspace.Tables = make(map[string]*TableMetadata)
 	for i := range tables {
@@ -290,6 +325,11 @@ func compileMetadata(
 	for i := range views {
 		keyspace.Views[views[i].Name] = &views[i]
 	}
+	keyspace.MaterializedViews = make(map[string]*MaterializedViewMetadata, len(materializedViews))
+	for _, materializedView := range materializedViews {
+		materializedView.BaseTable = keyspace.Tables[materializedView.baseTableName]
+		keyspace.MaterializedViews[materializedView.Name] = &materializedView
+	}
 
 	// add columns from the schema data
 	for i := range columns {
@@ -912,6 +952,75 @@ func getViewsMetadata(session *Session, keyspaceName string) ([]ViewMetadata, er
 	return views, nil
 }
 
+func getMaterializedViewsMetadata(session *Session, keyspaceName string) ([]MaterializedViewMetadata, error) {
+	if !session.useSystemSchema {
+		return nil, nil
+	}
+	var tableName = "system_schema.views"
+	stmt := fmt.Sprintf(`
+		SELECT
+			view_name,
+			base_table_id,
+			base_table_name,
+			bloom_filter_fp_chance,
+			caching,
+			comment,
+			compaction,
+			compression,
+			crc_check_chance,
+			dclocal_read_repair_chance,
+			default_time_to_live,
+			extensions,
+			gc_grace_seconds,
+			id,
+			include_all_columns,
+			max_index_interval,
+			memtable_flush_period_in_ms,
+			min_index_interval,
+			read_repair_chance,
+			speculative_retry
+		FROM %s
+		WHERE keyspace_name = ?`, tableName)
+
+	var materializedViews []MaterializedViewMetadata
+
+	rows := session.control.query(stmt, keyspaceName).Scanner()
+	for rows.Next() {
+		materializedView := MaterializedViewMetadata{Keyspace: keyspaceName}
+		err := rows.Scan(&materializedView.Name,
+			&materializedView.BaseTableId,
+			&materializedView.baseTableName,
+			&materializedView.BloomFilterFpChance,
+			&materializedView.Caching,
+			&materializedView.Comment,
+			&materializedView.Compaction,
+			&materializedView.Compression,
+			&materializedView.CrcCheckChance,
+			&materializedView.DcLocalReadRepairChance,
+			&materializedView.DefaultTimeToLive,
+			&materializedView.Extensions,
+			&materializedView.GcGraceSeconds,
+			&materializedView.Id,
+			&materializedView.IncludeAllColumns,
+			&materializedView.MaxIndexInterval,
+			&materializedView.MemtableFlushPeriodInMs,
+			&materializedView.MinIndexInterval,
+			&materializedView.ReadRepairChance,
+			&materializedView.SpeculativeRetry,
+		)
+		if err != nil {
+			return nil, err
+		}
+		materializedViews = append(materializedViews, materializedView)
+	}
+
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+
+	return materializedViews, nil
+}
+
 func getFunctionsMetadata(session *Session, keyspaceName string) ([]FunctionMetadata, error) {
 	if session.cfg.ProtoVersion == protoVersion1 || !session.hasAggregatesAndFunctions {
 		return nil, nil


Test patch:
diff --git a/cassandra_test.go b/cassandra_test.go
index 20e47b7..64ebf4e 100644
--- a/cassandra_test.go
+++ b/cassandra_test.go
@@ -325,12 +325,12 @@ func TestPagingWithBind(t *testing.T) {
 		t.Fatal("create table:", err)
 	}
 	for i := 0; i < 100; i++ {
-		if err := session.Query("INSERT INTO paging_bind (id,val) VALUES (?,?)", 1,i).Exec(); err != nil {
+		if err := session.Query("INSERT INTO paging_bind (id,val) VALUES (?,?)", 1, i).Exec(); err != nil {
 			t.Fatal("insert:", err)
 		}
 	}
 
-	q := session.Query("SELECT val FROM paging_bind WHERE id = ? AND val < ?",1, 50).PageSize(10)
+	q := session.Query("SELECT val FROM paging_bind WHERE id = ? AND val < ?", 1, 50).PageSize(10)
 	iter := q.Iter()
 	var id int
 	count := 0
@@ -2233,6 +2233,50 @@ func TestViewMetadata(t *testing.T) {
 	}
 }
 
+func TestMaterializedViewMetadata(t *testing.T) {
+	if flagCassVersion.Before(3, 0, 0) {
+		return
+	}
+	session := createSession(t)
+	defer session.Close()
+	createMaterializedViews(t, session)
+
+	materializedViews, err := getMaterializedViewsMetadata(session, "gocql_test")
+	if err != nil {
+		t.Fatalf("failed to query view metadata with err: %v", err)
+	}
+	if materializedViews == nil {
+		t.Fatal("failed to query view metadata, nil returned")
+	}
+	if len(materializedViews) != 1 {
+		t.Fatal("expected one view")
+	}
+	expectedView := MaterializedViewMetadata{
+		Keyspace:                "gocql_test",
+		Name:                    "view_view",
+		baseTableName:           "view_table",
+		BloomFilterFpChance:     0.01,
+		Caching:                 map[string]string{"keys": "ALL", "rows_per_partition": "NONE"},
+		Comment:                 "",
+		Compaction:              map[string]string{"class": "org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy", "max_threshold": "32", "min_threshold": "4"},
+		Compression:             map[string]string{"chunk_length_in_kb": "64", "class": "org.apache.cassandra.io.compress.LZ4Compressor"},
+		CrcCheckChance:          1,
+		DcLocalReadRepairChance: 0.1,
+		DefaultTimeToLive:       0,
+		Extensions:              map[string]string{},
+		GcGraceSeconds:          864000,
+		IncludeAllColumns:       false, MaxIndexInterval: 2048, MemtableFlushPeriodInMs: 0, MinIndexInterval: 128, ReadRepairChance: 0,
+		SpeculativeRetry: "99PERCENTILE",
+	}
+
+	expectedView.BaseTableId = materializedViews[0].BaseTableId
+	expectedView.Id = materializedViews[0].Id
+
+	if !reflect.DeepEqual(materializedViews[0], expectedView) {
+		t.Fatalf("materialized view is %+v, but expected %+v", materializedViews[0], expectedView)
+	}
+}
+
 func TestAggregateMetadata(t *testing.T) {
 	session := createSession(t)
 	defer session.Close()
@@ -2363,6 +2407,7 @@ func TestKeyspaceMetadata(t *testing.T) {
 	}
 	createAggregate(t, session)
 	createViews(t, session)
+	createMaterializedViews(t, session)
 
 	if err := session.Query("CREATE INDEX index_metadata ON test_metadata ( third_id )").Exec(); err != nil {
 		t.Fatalf("failed to create index with err: %v", err)
@@ -2433,6 +2478,15 @@ func TestKeyspaceMetadata(t *testing.T) {
 	if !found {
 		t.Fatal("failed to find the view in metadata")
 	}
+	if flagCassVersion.Major >= 3 {
+		materializedView, found := keyspaceMetadata.MaterializedViews["view_view"]
+		if !found {
+			t.Fatal("failed to find the materialized view in metadata")
+		}
+		if materializedView.BaseTable.Name != "view_table" {
+			t.Fatalf("expected name: %s, materialized view base table name: %s", "view_table", materializedView.BaseTable.Name)
+		}
+	}
 }
 
 // Integration test of the routing key calculation
diff --git a/common_test.go b/common_test.go
index f071453..c7b912f 100644
--- a/common_test.go
+++ b/common_test.go
@@ -184,6 +184,26 @@ func createViews(t *testing.T, session *Session) {
 	}
 }
 
+func createMaterializedViews(t *testing.T, session *Session) {
+	if flagCassVersion.Before(3, 0, 0) {
+		return
+	}
+	if err := session.Query(`CREATE TABLE IF NOT EXISTS gocql_test.view_table (
+		    userid text,
+		    year int,
+		    month int,
+    		    PRIMARY KEY (userid));`).Exec(); err != nil {
+		t.Fatalf("failed to create materialized view with err: %v", err)
+	}
+	if err := session.Query(`CREATE MATERIALIZED VIEW IF NOT EXISTS gocql_test.view_view AS
+		   SELECT year, month, userid
+		   FROM gocql_test.view_table
+		   WHERE year IS NOT NULL AND month IS NOT NULL AND userid IS NOT NULL
+		   PRIMARY KEY (userid, year);`).Exec(); err != nil {
+		t.Fatalf("failed to create materialized view with err: %v", err)
+	}
+}
+
 func createFunctions(t *testing.T, session *Session) {
 	if err := session.Query(`
 		CREATE OR REPLACE FUNCTION gocql_test.avgState ( state tuple<int,bigint>, val int )
diff --git a/metadata_test.go b/metadata_test.go
index 61b2ecf..c874864 100644
--- a/metadata_test.go
+++ b/metadata_test.go
@@ -94,7 +94,7 @@ func TestCompileMetadata(t *testing.T) {
 		{Keyspace: "V1Keyspace", Table: "peers", Kind: ColumnRegular, Name: "schema_version", ComponentIndex: 0, Validator: "org.apache.cassandra.db.marshal.UUIDType"},
 		{Keyspace: "V1Keyspace", Table: "peers", Kind: ColumnRegular, Name: "tokens", ComponentIndex: 0, Validator: "org.apache.cassandra.db.marshal.SetType(org.apache.cassandra.db.marshal.UTF8Type)"},
 	}
-	compileMetadata(1, keyspace, tables, columns, nil, nil, nil)
+	compileMetadata(1, keyspace, tables, columns, nil, nil, nil, nil)
 	assertKeyspaceMetadata(
 		t,
 		keyspace,
@@ -375,7 +375,7 @@ func TestCompileMetadata(t *testing.T) {
 			Validator: "org.apache.cassandra.db.marshal.UTF8Type",
 		},
 	}
-	compileMetadata(2, keyspace, tables, columns, nil, nil, nil)
+	compileMetadata(2, keyspace, tables, columns, nil, nil, nil, nil)
 	assertKeyspaceMetadata(
 		t,
 		keyspace,