Implement late binding views in redshift by default (#499)

* Implement late binding views in redshift by default to avoid possible transaction conflicts * Review comment * Increment version to 1.4.1
dataform-co · Nov 28, 2019 · cd618a6 · cd618a6
1 parent 1e7084b
commit cd618a6
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 42 deletions.
diff --git a/core/adapters/redshift.ts b/core/adapters/redshift.ts
@@ -63,9 +63,12 @@ export class RedshiftAdapter extends Adapter implements IAdapter {
       .add(Task.assertion(`select sum(1) as row_count from ${this.resolveTarget(target)}`));
   }
 
-  public createOrReplaceView(target: dataform.ITarget, query: string) {
-    return `
-      create or replace view ${this.resolveTarget(target)} as ${query}`;
+  public createOrReplaceView(target: dataform.ITarget, query: string, bind = false) {
+    const createQuery = `create or replace view ${this.resolveTarget(target)} as ${query}`;
+    if (bind) {
+      return createQuery;
+    }
+    return `${createQuery} with no schema binding`;
   }
 
   public createOrReplace(table: dataform.ITable) {
@@ -75,9 +78,13 @@ export class RedshiftAdapter extends Adapter implements IAdapter {
           // Drop the view in case we are changing the number of column(s) (or their types).
           .add(Task.statement(this.dropIfExists(table.target, this.baseTableType(table.type))))
           .add(
-            Task.statement(`
-        create or replace view ${this.resolveTarget(table.target)}
-        as ${table.query}`)
+            Task.statement(
+              this.createOrReplaceView(
+                table.target,
+                table.query,
+                table.redshift && table.redshift.bind
+              )
+            )
           )
       );
     }

diff --git a/core/utils.ts b/core/utils.ts
@@ -140,21 +140,21 @@ export function validate(compiledGraph: dataform.ICompiledGraph): dataform.IGrap
 
     // redshift config
     if (!!action.redshift) {
-      if (
-        Object.keys(action.redshift).length === 0 ||
-        Object.values(action.redshift).every((value: string) => !value.length)
-      ) {
-        const message = `Missing properties in redshift config`;
-        validationErrors.push(dataform.ValidationError.create({ message, actionName }));
-      }
-
       const validatePropertyDefined = (
         opts: dataform.IRedshiftOptions,
         prop: keyof dataform.IRedshiftOptions
       ) => {
-        if (!opts[prop] || !opts[prop].length) {
-          const message = `Property "${prop}" is not defined`;
-          validationErrors.push(dataform.ValidationError.create({ message, actionName }));
+        const error = dataform.ValidationError.create({
+          message: `Property "${prop}" is not defined`,
+          actionName
+        });
+        const value = opts[prop];
+        if (!opts.hasOwnProperty(prop)) {
+          validationErrors.push(error);
+        } else if (value instanceof Array) {
+          if (value.length === 0) {
+            validationErrors.push(error);
+          }
         }
       };
       const validatePropertiesDefined = (

diff --git a/docs/pages/guides/warehouses/redshift.mdx b/docs/pages/guides/warehouses/redshift.mdx
@@ -46,3 +46,17 @@ config {
 }
 SELECT 1 AS ts
 ```
+
+# Binding views
+
+By default, all views in Redshift are created as late binding views. This can be changed by setting the `bind` property in the redshift configuration block.
+
+```js
+config {
+  type: "view",
+  redshift: {
+    bind: true
+  }
+}
+SELECT 1 AS ts
+```
diff --git a/protos/core.proto b/protos/core.proto
@@ -88,6 +88,7 @@ message RedshiftOptions {
   string dist_style = 2;
   repeated string sort_keys = 3;
   string sort_style = 4;
+  bool bind = 5;
 }
 
 message SQLDataWarehouseOptions {

diff --git a/tests/api/api.spec.ts b/tests/api/api.spec.ts
@@ -357,6 +357,27 @@ describe("@dataform/api", () => {
               name: "redshift_without_redshift"
             },
             query: "query"
+          },
+          {
+            name: "redshift_view",
+            type: "view",
+            target: {
+              schema: "schema",
+              name: "redshift_view"
+            },
+            query: "query"
+          },
+          {
+            name: "redshift_view_with_binding",
+            type: "view",
+            target: {
+              schema: "schema",
+              name: "redshift_view_with_binding"
+            },
+            query: "query",
+            redshift: {
+              bind: true
+            }
           }
         ]
       });
@@ -365,15 +386,17 @@ describe("@dataform/api", () => {
         'create table "schema"."redshift_all_temp" diststyle even distkey (column1) compound sortkey (column1, column2) as query',
         'create table "schema"."redshift_only_sort_temp" interleaved sortkey (column1) as query',
         'create table "schema"."redshift_only_dist_temp" diststyle even distkey (column1) as query',
-        'create table "schema"."redshift_without_redshift_temp" as query'
+        'create table "schema"."redshift_without_redshift_temp" as query',
+        'create or replace view "schema"."redshift_view" as query with no schema binding',
+        'create or replace view "schema"."redshift_view_with_binding" as query'
       ];
 
       const builder = new Builder(testGraph, {}, testState);
       const executionGraph = builder.build();
 
       expect(executionGraph.actions)
         .to.be.an("array")
-        .to.have.lengthOf(4);
+        .to.have.lengthOf(6);
 
       executionGraph.actions.forEach((action, index) => {
         expect(action)

diff --git a/tests/core/core.spec.ts b/tests/core/core.spec.ts
@@ -283,35 +283,32 @@ describe("@dataform/core", () => {
           sortStyle: "wrong_sortStyle"
         }
       });
-      session.publish("example_empty_redshift", {
-        redshift: {}
-      });
 
       const expectedResults = [
-        { name: "schema.example_absent_distKey", message: /Property "distKey" is not defined/ },
-        { name: "schema.example_absent_distStyle", message: /Property "distStyle" is not defined/ },
-        { name: "schema.example_wrong_distStyle", message: /Wrong value of "distStyle" property/ },
-        { name: "schema.example_absent_sortKeys", message: /Property "sortKeys" is not defined/ },
-        { name: "schema.example_empty_sortKeys", message: /Property "sortKeys" is not defined/ },
-        { name: "schema.example_absent_sortStyle", message: /Property "sortStyle" is not defined/ },
-        { name: "schema.example_wrong_sortStyle", message: /Wrong value of "sortStyle" property/ },
-        { name: "schema.example_empty_redshift", message: /Missing properties in redshift config/ }
+        { name: "schema.example_absent_distKey", message: `Property "distKey" is not defined` },
+        { name: "schema.example_absent_distStyle", message: `Property "distStyle" is not defined` },
+        {
+          name: "schema.example_wrong_distStyle",
+          message: `Wrong value of "distStyle" property. Should only use predefined values: "even" | "key" | "all"`
+        },
+        { name: "schema.example_absent_sortKeys", message: `Property "sortKeys" is not defined` },
+        { name: "schema.example_empty_sortKeys", message: `Property "sortKeys" is not defined` },
+        { name: "schema.example_absent_sortStyle", message: `Property "sortStyle" is not defined` },
+        {
+          name: "schema.example_wrong_sortStyle",
+          message: `Wrong value of "sortStyle" property. Should only use predefined values: "compound" | "interleaved"`
+        }
       ];
 
       const graph = session.compile();
       const gErrors = utils.validate(graph);
 
-      expect(gErrors)
-        .to.have.property("validationErrors")
-        .to.be.an("array")
-        .to.have.lengthOf(8);
-
-      expectedResults.forEach(result => {
-        const err = gErrors.validationErrors.find(e => e.actionName === result.name);
-        expect(err)
-          .to.have.property("message")
-          .that.matches(result.message);
-      });
+      expect(
+        gErrors.validationErrors.map(validationError => ({
+          name: validationError.actionName,
+          message: validationError.message
+        }))
+      ).to.have.deep.members(expectedResults);
     });
 
     it("validation_type_inline", () => {

diff --git a/version.bzl b/version.bzl
@@ -1,3 +1,3 @@
 # NOTE: If you change the format of this line, you must change the bash command
 # in /scripts/publish to extract the version string correctly.
-DF_VERSION = "1.4.0"
+DF_VERSION = "1.4.1"