WIP

Author: jvanstraten

Cargo.lock

@@ -211,6 +211,25 @@ message ExtensionDefinition {
       WindowProperties window_function = 9;
     }
 
+    // For table functions, when defined; to be added to above oneof.
+    reserved 10;
+
+    // Nullability behavior that the function was declared with. Note that the
+    // patterns have already been desugared to represent this, so this can be
+    // ignored; it exists only for completeness.
+    oneof consistency {
+      // Nullability of toplevel binding and data type patterns of all argument
+      // and return types was overridden to `??nullable`.
+      google.protobuf.Empty mirror = 11;
+
+      // Nullability of toplevel binding and data type patterns of all argument
+      // types was overridden to `??nullable`.
+      google.protobuf.Empty declared_output = 12;
+
+      // No desugaring overrides were applied.
+      google.protobuf.Empty discrete = 13;
+    }
+
     // Properties common to aggregate and window functions.
     message AggregateProperties {
       // When specified, the function is decomposable.
@@ -250,9 +269,8 @@ message ExtensionDefinition {
     }
   }
 
-  // Represents a parameter pack for a user-defined compound type class or a
-  // function argument slot list. In the latter case, the patterns will only
-  // ever be passed typenames.
+  // Represents a positional parameter pack for a user-defined compound type
+  // class or a function argument slot list.
   //
   // The order of operations for the various patterns is:
   //
@@ -303,7 +321,7 @@ message ExtensionDefinition {
         // for aggregate and window functions.
         google.protobuf.Empty literal = 6;
 
-        // An data value must be bound to the slot. This is done by means of
+        // A data value must be bound to the slot. This is done by means of
         // binding an expression, but the expression can always be evaluated or
         // reduced before the function is invoked. This is used for value
         // function arguments that are not marked as constant. The data type of
@@ -350,6 +368,18 @@ message ExtensionDefinition {
       // The maximum number of arguments that can be bound to the slot. Zero
       // is treated as unspecified/no upper limit.
       uint64 maximum = 2;
+
+      // Consistency that the variadic slot was declared with. Note that the
+      // patterns have already been desugared to represent this, so this can be
+      // ignored; it exists only for completeness.
+      oneof consistency {
+        // No desugaring overrides were applied.
+        google.protobuf.Empty consistent = 3;
+
+        // All consistent bindings in the last argument slot were overridden to
+        // inconsistent bindings.
+        google.protobuf.Empty inconsistent = 4;
+      }
     }
 
     // Optional additional constraints to apply when determining whether a

proto/substrait/validator/simple_extensions.proto

@@ -40,7 +40,7 @@ serde_yaml = "0.9"
 # both the schema and the input, so we need to depend on that as well, even
 # though we don't actually do any JSON serialization and deserialization.
 jsonschema = { version = "=0.15.0", default-features = false }
-serde_json = "1"
+serde_json = { version = "1", features = ["preserve_order"] }
 
 # Used for checking identifier syntax (could be removed if regexes don't end up
 # being useful elsewhere too).

rs/Cargo.toml

@@ -353,25 +353,6 @@ impl From<&extension::simple::type_class::Reference> for validator::data_type::U
     }
 }
 
-impl From<&extension::simple::type_class::Definition>
-    for validator::data_type::user_defined_type::Definition
-{
-    fn from(node: &extension::simple::type_class::Definition) -> Self {
-        Self {
-            structure: node
-                .structure
-                .iter()
-                .map(
-                    |(name, simple)| validator::data_type::user_defined_type::Element {
-                        name: name.to_string(),
-                        kind: simple.into(),
-                    },
-                )
-                .collect(),
-        }
-    }
-}
-
 impl From<&data::Variation> for validator::data_type::Variation {
     fn from(node: &data::Variation) -> Self {
         match node {

rs/src/export/proto.rs

@@ -246,6 +246,9 @@ pub enum Classification {
     #[strum(props(HiddenDescription = "invalid compound vs. simple function name usage"))]
     LinkCompoundVsSimpleFunctionName = 3010,
 
+    #[strum(props(Description = "discouraged name"))]
+    LinkDiscouragedName = 3011,
+
     // Type-related diagnostics (group 4).
     #[strum(props(HiddenDescription = "type-related diagnostics"))]
     Type = 4000,

rs/src/output/diagnostic.rs

@@ -531,6 +531,22 @@ impl<T> ResolutionResult<T> {
         self.expect(parse_context, if_not_applicable, |_, _| true, true, false)
     }
 
+    /// Emits an error if one or more definitions were found for this name
+    /// resolution, to be used just before defining a new item.
+    pub fn expect_not_yet_defined(&self, parse_context: &mut context::Context) {
+        if !self.visible.is_empty() {
+            traversal::push_diagnostic(
+                parse_context,
+                diagnostic::Level::Error,
+                cause!(
+                    LinkDuplicateDefinition,
+                    "{} is already defined",
+                    self.unresolved_reference
+                ),
+            );
+        }
+    }
+
     /// Silently returns the first matching item, if any. If there are none,
     /// this just returns an unresolved reference. Use
     /// filter_items().expect_one() to formulate error messages if there are
@@ -567,18 +583,4 @@ impl<T> ResolutionResult<T> {
             .next()
             .flatten()
     }
-
-    /// Return an error if one or more definitions were found for this name
-    /// resolution, to be used just before defining a new item.
-    pub fn expect_not_yet_defined(&self) -> diagnostic::Result<()> {
-        if self.visible.is_empty() {
-            Ok(())
-        } else {
-            Err(cause!(
-                LinkDuplicateDefinition,
-                "{} is already defined",
-                self.unresolved_reference
-            ))
-        }
-    }
 }

rs/src/output/extension/namespace.rs

@@ -6,10 +6,11 @@ use crate::output::path;
 use crate::util;
 use std::sync::Arc;
 
-/// Represents an identifier that was used to reference something. It is
-/// stored along with a resolution result to retain information about the
-/// reference even if the resolution failed, and is generally only used for
-/// identity/equality checks and diagnostic information.
+/// Represents an identifier that was used to reference an extension at the
+/// protobuf level. It is stored along with a resolution result to retain
+/// information about the reference even if the resolution failed, and is
+/// generally only used for identity/equality checks and diagnostic
+/// information.
 #[derive(Clone, Debug, Default)]
 pub struct Identifier {
     /// The name of the object being referred to, if known. Always stored using

rs/src/output/extension/reference.rs

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: Apache-2.0
+
+//! Module for the common types involved with representing extension
+//! definitions.
+
+/// Identifying information associated with an extension, that can be used to
+/// refer to the extension from elsewhere.
+#[derive(Clone, Debug)]
+pub struct Identifier {
+    /// The URI that the extension was declared with. Matched case-sensitively.
+    pub uri: String,
+
+    /// One or more aliases for this extension within the scope of the URI.
+    /// Matched case-insensitively.
+    pub names: Vec<String>,
+
+    /// Unique number for the extension, generated during traversal. The
+    /// number is only unique within the scope of a single run of the
+    /// validator, and may change between runs.
+    pub extension_id: u64,
+}
+
+/// Non-functional metadata common to all extension types.
+#[derive(Clone, Debug, Default)]
+pub struct Metadata {
+    // Optional description of the extension. Only serves as documentation.
+    pub description: String,
+}

rs/src/output/extension/simple/common.rs

@@ -2,6 +2,7 @@
 
 //! Module for representing simple extensions.
 
+pub mod common;
 pub mod function;
 pub mod module;
 pub mod type_class;

rs/src/output/extension/simple/mod.rs

@@ -72,14 +72,13 @@ impl<T: Scope> DynScope for T {
 }
 
 /// A parsed simple extension module/file.
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug)]
 pub struct Definition {
-    /// Unique number within the tree that can be used to refer to this
-    /// extension when exporting in protobuf form.
-    pub extension_id: u64,
+    /// Identifier for the extension.
+    pub identifier: extension::simple::common::Identifier,
 
-    /// Description of the module.
-    pub description: String,
+    /// Common metadata for the extension.
+    pub metadata: extension::simple::common::Metadata,
 
     /// The URI that was actually used to resolve the module.
     pub actual_uri: String,