Merge branch 'main' into dbt-dremio-dev-containers

.github/PULL_REQUEST_TEMPLATE.md

@@ -14,6 +14,12 @@
 
 -   [ ] Added a summary of what this PR accomplishes to CHANGELOG.md
 
+### Contributor License Agreement
+
+<!--- Applicable for non Dremio employees and for first time contributors -->
+
+-   [ ] Please make sure you have signed our [Contributor License Agreement](https://www.dremio.com/legal/contributor-agreement/), which enables Dremio to distribute your contribution without restriction.
+
 ### Related Issue
 
 <!--- Link to issue where this is tracked -->

.github/expected_failures.txt

@@ -3,12 +3,6 @@ tests/functional/adapter/dbt_clone/test_dbt_clone.py::TestCloneNotPossibleDremio
 tests/functional/adapter/dremio_specific/test_drop_temp_table.py::TestDropTempTableDremio::test_drop_temp_table
 tests/functional/adapter/dremio_specific/test_schema_parsing.py::TestSchemaParsingDremio::test_schema_with_dots
 tests/functional/adapter/dremio_specific/test_verify_ssl.py::TestVerifyCertificateDremio::test_insecure_request_warning_not_exist
-tests/functional/adapter/grants/test_incremental_grants.py::TestIncrementalGrantsDremio::test_incremental_grants
-tests/functional/adapter/grants/test_invalid_grants.py::TestInvalidGrantsDremio::test_invalid_grants
-tests/functional/adapter/grants/test_model_grants.py::TestViewGrantsDremio::test_view_table_grants
-tests/functional/adapter/grants/test_model_grants.py::TestTableGrantsDremio::test_view_table_grants
-tests/functional/adapter/grants/test_seed_grants.py::TestSeedGrantsDremio::test_seed_grants
-tests/functional/adapter/grants/test_snapshot_grants.py::TestSnapshotGrantsDremio::test_snapshot_grants
 tests/functional/adapter/relation/test_get_relation_last_modified.py::TestGetLastRelationModified::test_get_last_relation_modified
 tests/functional/adapter/unit_testing/test_unit_testing.py::TestDremioUnitTestingTypes::test_unit_test_data_type
 tests/functional/adapter/unit_testing/test_unit_testing.py::TestDremioUnitTestCaseInsensitivity::test_case_insensitivity

.github/scripts/create_env_file.sh

@@ -16,6 +16,9 @@ DREMIO_DATABASE=dbt_test
 DBT_TEST_USER_1=dbt_test_user_1
 DBT_TEST_USER_2=dbt_test_user_2
 DBT_TEST_USER_3=dbt_test_user_3
+DBT_TEST_ROLE_1=dbt_test_role_1
+DBT_TEST_ROLE_2=dbt_test_role_2
+DREMIO_EDITION=community
 EOF
 
 echo ".env file created successfully."

CHANGELOG.md

@@ -3,6 +3,15 @@
 ## Changes
 
 - When naming reflections, if a `name` config is not set, the `alias` config parameter will be used instead. If also undefined, it will refer to the model name instead of using `Unnamed Reflection`
+- Grants can now be set for both users and for roles. A prefix was added to handle this, with `user:` and `role:` being the valid prefixes. For example, `user:dbt_test_user_1` and `role:dbt_test_role_1`. If no prefix is provided, defaults to user for backwards compatibility.
+- Moves the `raw_on_schema_change` variable back into scope for the config validator
+- Adds `BaseIncrementalOnSchemaChange` test to test_incremental.py
+- Changed logic for partitioning when materializing tables. Double quoting issue has been removed, now letting the user decide the quoting
+    - New example: `partition_by=['month("datetime_utc")']`
+## Features
+
+- [#259](https://github.com/dremio/dbt-dremio/pull/259) Added support for roles in grants
+- [#273](https://github.com/dremio/dbt-dremio/pull/273) Fix issue with on_schema_change config
 
 # dbt-dremio v1.8.1
 
@@ -27,7 +36,7 @@
   - Integration via REST API
 
 ## Features
- 
+
 -   [#250](https://github.com/dremio/dbt-dremio/pull/250) Implementation of wikis and tags feature
 -   [#250](https://github.com/dremio/dbt-dremio/pull/256) Reflections are now handled through the Rest API
 
@@ -55,7 +64,7 @@
 ## Changes
 
 -   [#199](https://github.com/dremio/dbt-dremio/issues/199) Populate PyPI's `long_description` with contents of `README.md`
--   [#167](https://github.com/dremio/dbt-dremio/issues/167) Remove parentheses surrounding views in the create_view_as macro. In more complex queries, the parentheses cause performance issues. 
+-   [#167](https://github.com/dremio/dbt-dremio/issues/167) Remove parentheses surrounding views in the create_view_as macro. In more complex queries, the parentheses cause performance issues.
 -   [#211](https://github.com/dremio/dbt-dremio/issues/211) Make fetching model data false by default. This improves performance where job results do not need to be populated.
 -   [#203](https://github.com/dremio/dbt-dremio/issues/203) Allow for dots in schema name, by surrounding in single and double quotes.
 -   [#193](https://github.com/dremio/dbt-dremio/issues/193) Fixes Reflection bug: The name argument to ref() must be a string, got <class 'jinja2.runtime.Undefined'>

dbt/adapters/dremio/impl.py

@@ -149,10 +149,12 @@ def standardize_grants_dict(self, grants_table: agate.Table) -> dict:
             # Just needed to change these two values to match Dremio cols
             grantee = row["grantee_id"]
             privilege = row["privilege"]
+            grantee_type = row["grantee_type"]
+
             if privilege in grants_dict.keys():
-                grants_dict[privilege].append(grantee)
+                grants_dict[privilege].append(f"{grantee_type}:{grantee}")
             else:
-                grants_dict.update({privilege: [grantee]})
+                grants_dict.update({privilege: [f"{grantee_type}:{grantee}"]})
         return grants_dict
 
     # This is for use in the test suite

dbt/include/dremio/macros/adapters/apply_grants.sql

@@ -16,6 +16,24 @@ limitations under the License.*/
     {{ return(False) }}
 {%- endmacro -%}
 
+{%- macro dremio__split_grantee(grantee) -%}
+    {%- set splitted = grantee.split(':') -%}
+
+    {%- if splitted | length < 2 -%}
+        {{ log("Deprecation warning: grants to users will soon require the user: prefix", info=True) }}
+        {{ return(("user", grantee)) }}
+    {%- else -%}
+        {%- set prefix = splitted[0] -%}
+        {%- set remainder = splitted[1:] | join(':') -%}
+
+        {%- if prefix not in ['user', 'role'] -%}
+            {% do exceptions.CompilationError("Invalid prefix. Use either user or role") %}
+        {%- endif -%}
+
+        {{ return((prefix, remainder)) }}
+    {%- endif -%}
+{%- endmacro -%}
+
 {% macro dremio__get_show_grant_sql(relation) %}
     {%- if relation.type == 'table' -%}
         {%- set relation_without_double_quotes = target.datalake ~ '.' ~ target.root_path ~ '.' ~ relation.identifier-%}
@@ -30,17 +48,23 @@ limitations under the License.*/
     {%- else -%}
          {% do exceptions.CompilationError("Invalid profile configuration: please only specify one of cloud_host or software_host in profiles.yml") %}
     {%- endif %}
-    SELECT privilege, grantee_id
+    SELECT privilege, grantee_type, grantee_id
         FROM {{privileges_table}}
         WHERE object_id='{{ relation_without_double_quotes }}'
 {% endmacro %}
 
 {%- macro dremio__get_grant_sql(relation, privilege, grantees) -%}
-    grant {{ privilege }} on {{relation.type}} {{ relation }} to user {{adapter.quote(grantees[0])}}
+    {%- set type, name = dremio__split_grantee(grantees[0]) %}
+
+    grant {{ privilege }} on {{ relation.type }} {{ relation }}
+    to {{ type }} {{ adapter.quote(name) }}
 {%- endmacro -%}
 
-{%- macro default__get_revoke_sql(relation, privilege, grantees) -%}
-    revoke {{ privilege }} on {{ relation.type }} {{ relation }} from user {{adapter.quote(grantees[0])}}
+{%- macro dremio__get_revoke_sql(relation, privilege, grantees) -%}
+    {%- set type, name = dremio__split_grantee(grantees[0]) %}
+
+    revoke {{ privilege }} on {{ relation.type }} {{ relation }}
+    from {{ type }} {{ adapter.quote(name) }}
 {%- endmacro -%}
 
 {% macro dremio__call_dcl_statements(dcl_statement_list) %}

dbt/include/dremio/macros/materializations/helpers.sql

@@ -19,10 +19,7 @@ limitations under the License.*/
       {%- set cols = [cols] -%}
     {%- endif -%}
     {{ label }} (
-    {%- for item in cols -%}
-      {{ adapter.quote(item) }}
-      {%- if not loop.last -%},{%- endif -%}
-    {%- endfor -%}
+    {{ cols | join(', ') }}
     )
   {%- endif %}
 {%- endmacro -%}

dbt/include/dremio/macros/materializations/incremental/incremental.sql

@@ -25,6 +25,7 @@ limitations under the License.*/
   -- configs
   {%- set unique_key = config.get('unique_key', validator=validation.any[list, basestring]) -%}
   {%- set full_refresh_mode = (should_full_refresh()) -%}
+  {%- set raw_on_schema_change = config.get('on_schema_change', validator=validation.any[basestring]) or 'ignore' -%}
   {%- set on_schema_change = incremental_validate_on_schema_change(raw_on_schema_change) -%}
 
   -- the temp_ and backup_ relations should not already exist in the database; get_relation
@@ -70,7 +71,6 @@ limitations under the License.*/
     {%- set file_format = dbt_dremio_validate_get_file_format(raw_file_format) -%}
     {%- set incremental_predicates = config.get('predicates', none) or config.get('incremental_predicates', none) -%}
     {%- set strategy = dbt_dremio_validate_get_incremental_strategy(incremental_strategy) -%}
-    {%- set raw_on_schema_change = config.get('on_schema_change', validator=validation.any[basestring]) or 'ignore' -%}
     {% set build_sql = dbt_dremio_get_incremental_sql(strategy, intermediate_relation, target_relation, dest_columns, unique_key) %}
 
   {% endif %}

tests/functional/adapter/basic/test_incremental.py

@@ -20,6 +20,9 @@
 from dbt.tests.adapter.incremental.test_incremental_merge_exclude_columns import (
     BaseMergeExcludeColumns,
 )
+from dbt.tests.adapter.incremental.test_incremental_on_schema_change import (
+    BaseIncrementalOnSchemaChange,
+)
 from tests.fixtures.profiles import unique_schema, dbt_profile_data
 from tests.utils.util import BUCKET, SOURCE
 from dbt.tests.util import run_dbt, relation_from_name, check_relations_equal
@@ -112,6 +115,9 @@ def test_incremental(self, project):
 class TestBaseIncrementalNotSchemaChange(BaseIncrementalNotSchemaChange):
     pass
 
+class TestIncrementalOnSchemaChange(BaseIncrementalOnSchemaChange):
+    pass
+
 
 class TestBaseMergeExcludeColumnsDremio(BaseMergeExcludeColumns):
     def get_test_fields(self, project, seed, incremental_model, update_sql_file):

tests/functional/adapter/grants/test_incremental_grants.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import pytest, os
+
 from dbt.tests.util import (
     run_dbt_and_capture,
     get_manifest,
@@ -26,7 +28,9 @@
     user2_incremental_model_schema_yml,
 )
 
+DREMIO_EDITION = os.getenv("DREMIO_EDITION")
 
+@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.")
 class TestIncrementalGrantsDremio(BaseGrantsDremio, BaseIncrementalGrants):
     # Define this here to use our modified version of relation_from_name
     def get_grants_on_relation(self, project, relation_name):
@@ -53,7 +57,7 @@ def test_incremental_grants(self, project, get_test_users):
         model_id = "model.test.my_incremental_model"
         model = manifest.nodes[model_id]
         assert model.config.materialized == "incremental"
-        expected = {select_privilege_name: [test_users[0]]}
+        expected = {select_privilege_name: ["user:" + test_users[0]]}
         self.assert_expected_grants_match_actual(
             project, "my_incremental_model", expected
         )
@@ -80,7 +84,7 @@ def test_incremental_grants(self, project, get_test_users):
         manifest = get_manifest(project.project_root)
         model = manifest.nodes[model_id]
         assert model.config.materialized == "incremental"
-        expected = {select_privilege_name: [test_users[1]]}
+        expected = {select_privilege_name: ["user:" + test_users[1]]}
         self.assert_expected_grants_match_actual(
             project, "my_incremental_model", expected
         )

tests/functional/adapter/grants/test_invalid_grants.py

@@ -12,13 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import pytest, os
 
 from dbt.tests.adapter.grants.test_invalid_grants import BaseInvalidGrants
 from tests.functional.adapter.grants.base_grants import BaseGrantsDremio
 from tests.utils.util import relation_from_name
 from dbt.tests.util import get_connection
 
+DREMIO_EDITION = os.getenv("DREMIO_EDITION")
 
+@pytest.mark.skipif(DREMIO_EDITION == "community", reason="Dremio only supports grants in EE/DC editions.")
 class TestInvalidGrantsDremio(BaseGrantsDremio, BaseInvalidGrants):
     def grantee_does_not_exist_error(self):
         return "Grant on catalog entity failed. User invalid_user does not exist."