fix: deactivate diagnostics of substrait-validator correctly.

In several places, some diagnostics of the `substrait-validator` were attempted to deactivate by specifying a minimum level that was higher that the maximum level. This does not work anymore with newer versions of the validator. If attempted, we get failed assertions with the message `min <= max`. This PR instead sets both minimum and maximum level to `info` in these cases, which resolves the broken assertions. Signed-off-by: Ingo Müller <[email protected]>
ingomueller-net · Nov 29, 2024 · 85736bd · 85736bd
1 parent 915c889
commit 85736bd
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 16 deletions.
diff --git a/substrait_consumer/context.py b/substrait_consumer/context.py
@@ -34,10 +34,10 @@ def produce_isthmus_substrait(sql_string, schema_list, validate=False):
     json_plan = json_formatter.printer().print_(plan)
     if validate:
         config = sv.Config()
-        config.override_diagnostic_level(1002, "warning", "info") # error
-        config.override_diagnostic_level(2001, "warning", "info") # warning
-        config.override_diagnostic_level(3005, "warning", "info") # warning
-        config.override_diagnostic_level(1, "warning", "info") # warning
+        config.override_diagnostic_level(1002, "info", "info")  # error
+        config.override_diagnostic_level(2001, "info", "info")  # warning
+        config.override_diagnostic_level(3005, "info", "info")  # warning
+        config.override_diagnostic_level(1, "info", "info")  # warning
         sv.check_plan_valid(json_plan, config)
     return json_plan
 

diff --git a/substrait_consumer/producers/datafusion_producer.py b/substrait_consumer/producers/datafusion_producer.py
@@ -49,12 +49,12 @@ def _produce_substrait(
         if validate:
             config = sv.Config()
             # Error: missing required protobuf field: struct
-            config.override_diagnostic_level(1002, "error", "info")
+            config.override_diagnostic_level(1002, "info", "info")
             # Warning: cannot automatically determine whether plan version
             # is compatible with the Substrait version
-            config.override_diagnostic_level(7, "warning", "info") # warning
+            config.override_diagnostic_level(7, "info", "info")  # warning
             # Error: URI reference
-            config.override_diagnostic_level(3001, "error", "info")
+            config.override_diagnostic_level(3001, "info", "info")
             sv.check_plan_valid(substrait_plan_bytes, config)
         substrait_proto.ParseFromString(substrait_plan_bytes)
 

diff --git a/substrait_consumer/producers/duckdb_producer.py b/substrait_consumer/producers/duckdb_producer.py
@@ -43,10 +43,10 @@ def _produce_substrait(
             config = sv.Config()
             # Warning: cannot automatically determine whether plan version
             # is compatible with the Substrait version
-            config.override_diagnostic_level(7, "warning", "info")  # warning
+            config.override_diagnostic_level(7, "info", "info")  # warning
             # Warning: did not attempt to resolve YAML: configured recursion
             # limit for URI resolution has been reached
-            config.override_diagnostic_level(2001, "warning", "info")
+            config.override_diagnostic_level(2001, "info", "info")
             sv.check_plan_valid(proto_bytes, config)
         duckdb_substrait_plan = self._db_connection.get_substrait_json(sql_query)
         proto_bytes = duckdb_substrait_plan.fetchone()[0]

diff --git a/substrait_consumer/tests/integration/test_tpch_plans_valid.py b/substrait_consumer/tests/integration/test_tpch_plans_valid.py
@@ -76,12 +76,12 @@ def test_isthmus_substrait_plans_valid(
         config = sv.Config()
         # Isthmus plan overrides
         # ValueError: Error at plan: missing required protobuf field: version (code 1002)
-        config.override_diagnostic_level(1002, "error", "info")
+        config.override_diagnostic_level(1002, "info", "info")
         # ValueError: Warning at plan.extension_uris[0].uri: did not attempt to resolve YAML:
         # configured recursion limit for URI resolution has been reached
-        config.override_diagnostic_level(2001, "warning", "info")
+        config.override_diagnostic_level(2001, "info", "info")
         # Warning. not yet implemented: matching function calls with their definitions
-        config.override_diagnostic_level(1, "warning", "info")
+        config.override_diagnostic_level(1, "info", "info")
 
         sv.check_plan_valid(substrait_query, config)
 
@@ -110,13 +110,13 @@ def test_duckdb_substrait_plans_valid(
 
         # Duckdb plan overrides
         # not yet implemented: typecast validation rules are not yet implemented
-        config.override_diagnostic_level(1, "warning", "info")
+        config.override_diagnostic_level(1, "info", "info")
         # function definition unavailable: cannot check validity of call
-        config.override_diagnostic_level(6003, "warning", "info")
+        config.override_diagnostic_level(6003, "info", "info")
         # Function Anchor to YAML file
-        config.override_diagnostic_level(3001, "error", "info")
+        config.override_diagnostic_level(3001, "info", "info")
         # too few field names
-        config.override_diagnostic_level(4003, "error", "info")
+        config.override_diagnostic_level(4003, "info", "info")
 
         # Format the sql query by inserting all the table names
         self.duckdb_producer.setup(self.db_connection, local_files, named_tables)