ref: remove signature modes (#670)

Signature modes are not necessary. They used to be necessary because we
had a handful of varying signatures for aggregations to flatten window
arguments, but that was since removed.

crates/sparrow-compiler/src/dfg.rs

@@ -264,7 +264,7 @@ impl Dfg {
                         );
                     }
                     Expression::Inst(InstKind::Simple(op)) => op
-                        .signature(sparrow_instructions::Mode::Plan)
+                        .signature()
                         .assert_valid_argument_count(children.len() - 1),
                     Expression::Inst(InstKind::FieldRef) => {
                         anyhow::ensure!(

crates/sparrow-compiler/src/dfg/const_eval.rs

@@ -138,12 +138,7 @@ pub(super) fn evaluate_constant(
 
     let argument_types = args.iter().map(|i| i.data_type.clone().into()).collect();
     let argument_literals: Vec<_> = inputs.into_iter().map(Some).collect();
-    let result_type = typecheck_inst(
-        kind,
-        argument_types,
-        &argument_literals,
-        sparrow_instructions::Mode::Dfg,
-    )?;
+    let result_type = typecheck_inst(kind, argument_types, &argument_literals)?;
     let result_type = result_type.arrow_type().with_context(|| {
         format!(
             "Expected result of literal instruction to have concrete type, but got {result_type:?}"
@@ -219,13 +214,7 @@ mod tests {
             }
 
             let kind = InstKind::Simple(inst_op);
-            let inputs = vec![
-                ScalarValue::Null;
-                inst_op
-                    .signature(sparrow_instructions::Mode::Dfg)
-                    .parameters()
-                    .len()
-            ];
+            let inputs = vec![ScalarValue::Null; inst_op.signature().parameters().len()];
 
             if let Err(e) = super::evaluate_constant(&kind, inputs) {
                 println!("Failed to evaluate '{inst_op}': {e:?}");

crates/sparrow-compiler/src/plan/expression_to_plan.rs

@@ -2,7 +2,7 @@ use anyhow::Context;
 use sparrow_api::kaskada::v1alpha::DataType;
 use sparrow_api::kaskada::v1alpha::{expression_plan, ExpressionPlan};
 use sparrow_arrow::scalar_value::ScalarValue;
-use sparrow_instructions::{InstKind, Mode};
+use sparrow_instructions::InstKind;
 use sparrow_syntax::{ArgVec, FenlType};
 
 use crate::dfg::Expression;
@@ -97,11 +97,7 @@ fn infer_result_type(
         }
     }
 
-    let result_type = crate::types::instruction::typecheck_inst(
-        inst_kind,
-        argument_types,
-        &argument_literals,
-        Mode::Plan,
-    )?;
+    let result_type =
+        crate::types::instruction::typecheck_inst(inst_kind, argument_types, &argument_literals)?;
     DataType::try_from(&result_type).context("unable to encode result type")
 }

crates/sparrow-compiler/src/types/instruction.rs

@@ -5,7 +5,7 @@ use arrow::datatypes::{DataType, Field};
 use itertools::{izip, Itertools};
 use sparrow_arrow::scalar_value::ScalarValue;
 use sparrow_instructions::CastEvaluator;
-use sparrow_instructions::{InstKind, Mode};
+use sparrow_instructions::InstKind;
 use sparrow_syntax::{ArgVec, Collection, FenlType, Resolved};
 
 use crate::types::inference::validate_instantiation;
@@ -20,11 +20,10 @@ pub(crate) fn typecheck_inst(
     inst: &InstKind,
     argument_types: ArgVec<FenlType>,
     argument_literals: &[Option<ScalarValue>],
-    mode: Mode,
 ) -> anyhow::Result<FenlType> {
     match inst {
         InstKind::Simple(instruction) => {
-            let signature = instruction.signature(mode);
+            let signature = instruction.signature();
             let argument_types = Resolved::new(
                 Cow::Borrowed(signature.parameters().names()),
                 argument_types,

crates/sparrow-instructions/src/inst.rs

@@ -10,14 +10,6 @@ use strum::EnumProperty;
 
 use crate::value::ValueRef;
 
-/// The mode an instruction is being used in.
-///
-/// Affects which signature is used.
-pub enum Mode {
-    Dfg,
-    Plan,
-}
-
 /// Enumeration of the instruction operations.
 ///
 /// Instructions don't include any "payload" -- they are an operation applied
@@ -223,15 +215,12 @@ impl InstOp {
         )
     }
 
-    pub fn signature(&self, mode: Mode) -> &'static Signature {
-        match mode {
-            Mode::Dfg => INST_OP_SIGNATURES[*self].dfg(),
-            Mode::Plan => INST_OP_SIGNATURES[*self].plan(),
-        }
+    pub fn signature(&self) -> &'static Signature {
+        &INST_OP_SIGNATURES[*self]
     }
 
     pub fn name(&self) -> &'static str {
-        self.signature(Mode::Dfg).name()
+        self.signature().name()
     }
 }
 
@@ -272,50 +261,19 @@ impl Inst {
     }
 }
 
-struct OpSignatures {
-    dfg: Arc<Signature>,
-    plan: Arc<Signature>,
-}
-
-impl OpSignatures {
-    pub fn dfg(&self) -> &Signature {
-        self.dfg.as_ref()
-    }
-
-    pub fn plan(&self) -> &Signature {
-        self.plan.as_ref()
-    }
-}
-
 #[dynamic]
-static INST_OP_SIGNATURES: EnumMap<InstOp, OpSignatures> = {
+static INST_OP_SIGNATURES: EnumMap<InstOp, Arc<Signature>> = {
     enum_map! {
         op => {
             let signature = parse_signature(op, "signature");
-            let dfg_signature = parse_signature(op, "dfg_signature");
-            let plan_signature = parse_signature(op, "plan_signature");
-
-            let signatures = match (signature, dfg_signature, plan_signature) {
-                (None, Some(dfg), Some(plan)) => {
-                    assert_eq!(dfg.name(), plan.name(), "Names for both DFG and Plan signature must be the same");
-                    OpSignatures { dfg, plan }
-                },
-                (Some(shared), None, None) => {
-                    OpSignatures {
-                        dfg: shared.clone(),
-                        plan: shared
-                    }
-                }
-                (shared, dfg, plan) => {
-                    // Must have either (shared) or (dfg and plan).
-                    panic!("Missing or invalid signatures for instruction {op:?}: shared={shared:?}, dfg={dfg:?}, plan={plan:?}")
-                }
+            let signature = match signature {
+                Some(s) => s,
+                None => panic!("Missing or invalid signatures for instruction {op:?}: signature={signature:?}")
             };
 
-            for parameter in signatures.plan().parameters().types() {
+            for parameter in signature.parameters().types() {
                 if matches!(parameter.inner(), FenlType::Window) {
-                    panic!("Illegal type '{}' in plan_signature for instruction {:?}", parameter.inner(), op)
-
+                    panic!("Illegal type '{}' in signature for instruction {:?}", parameter.inner(), op)
                 }
             }
 
@@ -324,10 +282,10 @@ static INST_OP_SIGNATURES: EnumMap<InstOp, OpSignatures> = {
             // make sure we don't accidentally introduce `InstOp` with
             // these names (since they are handled specially in the
             // planning / execution).
-            assert_ne!(signatures.dfg().name(), "record", "'record' is a reserved instruction name");
-            assert_ne!(signatures.dfg().name(), "field_ref", "'field_ref' is a reserved instruction name");
+            assert_ne!(signature.name(), "record", "'record' is a reserved instruction name");
+            assert_ne!(signature.name(), "field_ref", "'field_ref' is a reserved instruction name");
 
-            signatures
+            signature
         }
     }
 };