sql: add support for materialized views #52530

rohany · 2020-08-08T06:23:50Z

This commit adds support for materialized views in CockroachDB. These
materialized views follow the Postgres style, where the REFRESH MATERIALIZED VIEW command is needed to update the stored view data.
This commit does not include adding the various ALTER MATERIALIZED VIEW and DROP MATERIALIZED VIEW extensions and will be added in a
follow up PR.

Release note (sql change): Add support for materialized views.

cockroach-teamcity · 2020-08-08T06:23:57Z

This change is

jordanlewis

Wow, awesome work!

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru, @ajwerner, @jordanlewis, @RaduBerinde, and @rohany)

pkg/sql/resolver.go, line 392 at r1 (raw file):

		if desc != nil && desc.IsView() && !desc.MaterializedView() {
			// TODO (rohany): Return a pgerror here.
			return nil, nil, errors.New("its a viewwwww")

😠

pkg/sql/schema_changer.go, line 333 at r1 (raw file):

		return nil
	}
	log.Info(ctx, "starting backfill for CREATE TABLE AS")

add the table details to this message

pkg/sql/schema_changer.go, line 344 at r1 (raw file):

		return nil
	}
	log.Infof(ctx, "starting backfill for CREATE MATERIALIZED VIEW")

Add the view details to this message

pkg/sql/catalog/descpb/structured.proto, line 548 at r1 (raw file):

  optional IndexDescriptor new_primary_index = 1 [(gogoproto.nullable) = false];
  repeated IndexDescriptor new_indexes = 2 [(gogoproto.nullable) = false];
  optional util.hlc.Timestamp as_of = 3 [(gogoproto.nullable) = false];

Comments on these please

pkg/sql/catalog/descpb/structured.proto, line 870 at r1 (raw file):

  // a TableDescriptor represents a view.
  optional string view_query = 24 [(gogoproto.nullable) = false];
  optional bool is_materialized_view = 41 [(gogoproto.nullable) = false];

Comment on this (e.g. it should only be set if view_query is also set)

pkg/sql/logictest/testdata/logic_test/materialized_view, line 91 at r1 (raw file):


statement error pq: cannot mutate materialized view "v"
DELETE FROM v WHERE x = 1

What does TRUNCATE do?

rohany

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @adityamaru, @ajwerner, @jordanlewis, and @RaduBerinde)

pkg/sql/resolver.go, line 392 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

😠

woops

pkg/sql/schema_changer.go, line 333 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

add the table details to this message

Done. Added the query -- the table ID etc are already in the context's logtags

pkg/sql/schema_changer.go, line 344 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

Add the view details to this message

Done.

pkg/sql/catalog/descpb/structured.proto, line 548 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

Comments on these please

Done.

pkg/sql/catalog/descpb/structured.proto, line 870 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

Comment on this (e.g. it should only be set if view_query is also set)

Done.

pkg/sql/logictest/testdata/logic_test/materialized_view, line 91 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

What does TRUNCATE do?

Done, checked against the postgres error.

jordanlewis

pg_class.relkind needs to be updated to store m for materialized views.

, I think a review from a schema/bulkio focused person would be good. 🎉

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @adityamaru, @ajwerner, @RaduBerinde, and @rohany)

pkg/sql/refresh_materialized_view.go, line 83 at r2 (raw file):

		NewPrimaryIndex: *newPrimaryIndex,
		NewIndexes:      newIndexes,
		AsOf:            params.p.Txn().ReadTimestamp(),

Should we make this in the past to avoid contention?

rohany · 2020-08-12T16:00:54Z

, I think a review from a schema/bulkio focused person would be good. 🎉

Yeah, I want @ajwerner to take a peek

Should we make this in the past to avoid contention?

What do you mean -- we have to take the read timestamp of the actual call to refresh. By the time that the schema change runs it's in the past however.

RaduBerinde

Opt part LGTM

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @adityamaru, @ajwerner, @RaduBerinde, and @rohany)

ajwerner

mod disabling hash sharded indexes

Reviewed 10 of 41 files at r1, 5 of 6 files at r2, 29 of 29 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @adityamaru, @jordanlewis, and @rohany)

pkg/sql/create_index.go, line 67 at r3 (raw file):

			return nil, pgerror.New(pgcode.InvalidObjectDefinition,
				"cannot create interleaved index on materialized view")
		}

Can we disable hash sharded indexes here too? I don't think I want to deal with adding the column and they don't really make sense.

pkg/sql/refresh_materialized_view.go, line 83 at r2 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

Should we make this in the past to avoid contention?

Meh, I think it'd be weird to do that without the user telling us. Like say we do a write and then a refresh, I definitely want to see that write. I'd be cool with creating a follow-up item to support an AS OF SYSTEM TIME clause.

pkg/sql/refresh_materialized_view.go, line 47 at r3 (raw file):

		mut := &desc.Mutations[i]
		if mut.GetMaterializedViewRefresh() != nil {
			return nil, pgerror.Newf(pgcode.SQLStatementNotYetComplete, "view is already being refreshed")

nit: maybe pgcode.ObjectNotInPrerequisiteState instead?

pkg/sql/logictest/testdata/logic_test/materialized_view, line 44 at r3 (raw file):

query I rowsort
SELECT y FROM v WHERE y > 4

how do you know you're using it? Maybe force the use with v@i

Fixes cockroachdb#41649. This commit adds support for materialized views in CockroachDB. These materialized views follow the Postgres style, where the `REFRESH MATERIALIZED VIEW` command is needed to update the stored view data. This commit does not include adding the various `ALTER MATERIALIZED VIEW` and `DROP MATERIALIZED VIEW` extensions and will be added in a follow up PR. Release note (sql change): Add support for materialized views.

rohany

Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @adityamaru, @ajwerner, and @jordanlewis)

pkg/sql/create_index.go, line 67 at r3 (raw file):

Previously, ajwerner wrote…

Can we disable hash sharded indexes here too? I don't think I want to deal with adding the column and they don't really make sense.

Done.

pkg/sql/refresh_materialized_view.go, line 47 at r3 (raw file):

Previously, ajwerner wrote…

nit: maybe pgcode.ObjectNotInPrerequisiteState instead?

done

pkg/sql/logictest/testdata/logic_test/materialized_view, line 44 at r3 (raw file):

Previously, ajwerner wrote…

query I rowsort
SELECT y FROM v WHERE y > 4
how do you know you're using it? Maybe force the use with v@i

Done.

rohany · 2020-08-14T01:12:08Z

TFTRs!

bors r+

craig · 2020-08-14T01:34:14Z

Build succeeded:

Compile Build (Cockroach)

wzrdtales · 2020-09-11T16:51:55Z

Great, will materialized views carry support for indexes on them? That would be amazing, then cockroachdb could catch up in the analytical space even more by building multi dimensional indexed data representations (like in a data warehouse). Looking forward to this!

rohany requested review from jordanlewis, ajwerner and RaduBerinde August 8, 2020 06:23

rohany requested a review from a team as a code owner August 8, 2020 06:23

rohany force-pushed the materialized-views branch 7 times, most recently from 84e0e7b to 01cd314 Compare August 10, 2020 18:53

rohany requested a review from adityamaru August 10, 2020 18:53

jordanlewis reviewed Aug 10, 2020

View reviewed changes

rohany force-pushed the materialized-views branch from 01cd314 to ae886a5 Compare August 10, 2020 20:10

rohany commented Aug 10, 2020

View reviewed changes

jordanlewis approved these changes Aug 12, 2020

View reviewed changes

rohany force-pushed the materialized-views branch 2 times, most recently from ffa6470 to 49f5626 Compare August 12, 2020 16:39

rohany mentioned this pull request Aug 12, 2020

sql: implement ALTER MATERIALIZED VIEW and DROP MATERIALIZED VIEW #52733

Closed

RaduBerinde reviewed Aug 13, 2020

View reviewed changes

ajwerner approved these changes Aug 13, 2020

View reviewed changes

rohany force-pushed the materialized-views branch from 49f5626 to c362707 Compare August 13, 2020 23:37

rohany commented Aug 14, 2020

View reviewed changes

craig bot merged commit 1049eaa into cockroachdb:master Aug 14, 2020

lnhsingh mentioned this pull request Aug 26, 2020

sql: add support for materialized views cockroachdb/docs#8121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: add support for materialized views #52530

sql: add support for materialized views #52530

rohany commented Aug 8, 2020

cockroach-teamcity commented Aug 8, 2020

jordanlewis left a comment

rohany left a comment

jordanlewis left a comment

rohany commented Aug 12, 2020 •

edited by petermattis

Loading

RaduBerinde left a comment

ajwerner left a comment

rohany left a comment

rohany commented Aug 14, 2020

craig bot commented Aug 14, 2020

wzrdtales commented Sep 11, 2020

sql: add support for materialized views #52530

sql: add support for materialized views #52530

Conversation

rohany commented Aug 8, 2020

cockroach-teamcity commented Aug 8, 2020

jordanlewis left a comment

Choose a reason for hiding this comment

rohany left a comment

Choose a reason for hiding this comment

jordanlewis left a comment

Choose a reason for hiding this comment

rohany commented Aug 12, 2020 • edited by petermattis Loading

RaduBerinde left a comment

Choose a reason for hiding this comment

ajwerner left a comment

Choose a reason for hiding this comment

rohany left a comment

Choose a reason for hiding this comment

rohany commented Aug 14, 2020

craig bot commented Aug 14, 2020

wzrdtales commented Sep 11, 2020

rohany commented Aug 12, 2020 •

edited by petermattis

Loading