fix! revert TableRef changes (#1004)

# Rationale for this change

* #998 added a breaking change that #1001 attempted to resolve. However,
this fix is not enough to resolve some downstream issues. This PR is an
alternative, non-breaking, replacement to #998.

# What changes are included in this PR?

* revert the offending commits (first 2 commits)
*
6bdb70a
reverts
50de226
which is all of #1001
*
e928513
reverts
015b7e3
from #998
* rewrite the remaining code in #998 alternatively (last commit)

# Are these changes tested?
* Yes, existing tests pass, including the new test that #998 introduced
to cover the bug

Author: JayWhite2357

crates/proof-of-sql/src/base/database/table_ref.rs

@@ -11,21 +11,9 @@ use sqlparser::ast::Ident;
 
 /// Expression for an SQL table
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
-pub enum TableRef {
-    /// Fully qualified table reference with schema and table name
-    FullyQualified {
-        /// Schema name
-        schema_name: Ident,
-        /// Table name
-        table_name: Ident,
-    },
-    /// Table reference without schema
-    TableOnly {
-        /// Table name
-        table_name: Ident,
-    },
-    /// No table reference
-    None,
+pub struct TableRef {
+    schema_name: Option<Ident>,
+    table_name: Ident,
 }
 
 impl TableRef {
@@ -36,62 +24,45 @@ impl TableRef {
         let schema = schema_name.as_ref();
         let table = table_name.as_ref();
 
-        if schema.is_empty() {
-            Self::TableOnly {
-                table_name: Ident::new(table.to_string()),
-            }
-        } else {
-            Self::FullyQualified {
-                schema_name: Ident::new(schema.to_string()),
-                table_name: Ident::new(table.to_string()),
-            }
+        Self {
+            schema_name: if schema.is_empty() {
+                None
+            } else {
+                Some(Ident::new(schema.to_string()))
+            },
+            table_name: Ident::new(table.to_string()),
         }
     }
 
-    /// Returns the identifier of the schema if it exists. Otherwise returns `None`.
+    /// Returns the identifier of the schema
+    /// # Panics
     #[must_use]
     pub fn schema_id(&self) -> Option<&Ident> {
-        match self {
-            Self::FullyQualified { schema_name, .. } => Some(schema_name),
-            Self::TableOnly { .. } | Self::None => None,
-        }
+        self.schema_name.as_ref()
     }
 
-    /// Returns the identifier of the table if it exists. Otherwise returns `None`.
+    /// Returns the identifier of the table
+    /// # Panics
     #[must_use]
-    pub fn table_id(&self) -> Option<&Ident> {
-        match self {
-            Self::FullyQualified { table_name, .. } | Self::TableOnly { table_name } => {
-                Some(table_name)
-            }
-            Self::None => None,
-        }
+    pub fn table_id(&self) -> &Ident {
+        &self.table_name
     }
 
     /// Creates a new table reference from an optional schema and table name.
     #[must_use]
     pub fn from_names(schema_name: Option<&str>, table_name: &str) -> Self {
-        if let Some(schema) = schema_name {
-            Self::FullyQualified {
-                schema_name: Ident::new(schema.to_string()),
-                table_name: Ident::new(table_name.to_string()),
-            }
-        } else {
-            Self::TableOnly {
-                table_name: Ident::new(table_name.to_string()),
-            }
+        Self {
+            schema_name: schema_name.map(|s| Ident::new(s.to_string())),
+            table_name: Ident::new(table_name.to_string()),
         }
     }
 
     /// Creates a `TableRef` directly from `Option<Ident>` for schema and `Ident` for table.
     #[must_use]
     pub fn from_idents(schema_name: Option<Ident>, table_name: Ident) -> Self {
-        match schema_name {
-            Some(schema) => Self::FullyQualified {
-                schema_name: schema,
-                table_name,
-            },
-            None => Self::TableOnly { table_name },
+        Self {
+            schema_name,
+            table_name,
         }
     }
 
@@ -133,8 +104,8 @@ impl TryFrom<&str> for TableRef {
 /// Note: We just need this conversion trait until `SelectStatement` refactor is done
 impl From<ResourceId> for TableRef {
     fn from(id: ResourceId) -> Self {
-        Self::FullyQualified {
-            schema_name: Ident::from(id.schema()),
+        TableRef {
+            schema_name: Some(Ident::from(id.schema())),
             table_name: Ident::from(id.object_name()),
         }
     }
@@ -150,35 +121,16 @@ impl FromStr for TableRef {
 
 impl Equivalent<TableRef> for &TableRef {
     fn equivalent(&self, key: &TableRef) -> bool {
-        match (self, key) {
-            (
-                TableRef::FullyQualified {
-                    schema_name: s1,
-                    table_name: t1,
-                },
-                TableRef::FullyQualified {
-                    schema_name: s2,
-                    table_name: t2,
-                },
-            ) => s1 == s2 && t1 == t2,
-            (TableRef::TableOnly { table_name: t1 }, TableRef::TableOnly { table_name: t2 }) => {
-                t1 == t2
-            }
-            (TableRef::None, TableRef::None) => true,
-            _ => false,
-        }
+        self.schema_name == key.schema_name && self.table_name == key.table_name
     }
 }
 
 impl Display for TableRef {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            TableRef::FullyQualified {
-                schema_name,
-                table_name,
-            } => write!(f, "{}.{}", schema_name.value, table_name.value),
-            TableRef::TableOnly { table_name } => write!(f, "{}", table_name.value),
-            TableRef::None => write!(f, "<no_table>"),
+        if let Some(schema) = &self.schema_name {
+            write!(f, "{}.{}", schema.value, self.table_name.value)
+        } else {
+            write!(f, "{}", self.table_name.value)
         }
     }
 }

crates/proof-of-sql/src/sql/evm_proof_plan/exprs.rs

@@ -153,7 +153,7 @@ impl EVMColumnExpr {
                 .get_index_of(expr.column_ref())
                 .or_else(|| {
                     column_refs.get_index_of(&ColumnRef::new(
-                        TableRef::None,
+                        TableRef::from_names(None, ""),
                         expr.column_id(),
                         expr.data_type(),
                     ))

crates/proof-of-sql/src/sql/evm_proof_plan/plans.rs

@@ -241,7 +241,7 @@ impl EVMProjectionExec {
             .input()
             .get_column_result_fields()
             .into_iter()
-            .map(|f| ColumnRef::new(TableRef::None, f.name(), f.data_type()))
+            .map(|f| ColumnRef::new(TableRef::from_names(None, ""), f.name(), f.data_type()))
             .collect();
         Ok(Self {
             input_plan: Box::new(EVMDynProofPlan::try_from_proof_plan(
@@ -271,7 +271,7 @@ impl EVMProjectionExec {
         let input_result_column_refs = input
             .get_column_result_fields()
             .into_iter()
-            .map(|f| ColumnRef::new(TableRef::None, f.name(), f.data_type()))
+            .map(|f| ColumnRef::new(TableRef::from_names(None, ""), f.name(), f.data_type()))
             .collect();
         Ok(ProjectionExec::new(
             self.results
@@ -1192,12 +1192,12 @@ mod tests {
                     expr: DynProofExpr::Add(
                         AddExpr::try_new(
                             Box::new(DynProofExpr::Column(ColumnExpr::new(ColumnRef::new(
-                                TableRef::None,
+                                TableRef::from_names(None, ""),
                                 ident_a.clone(),
                                 ColumnType::BigInt,
                             )))),
                             Box::new(DynProofExpr::Column(ColumnExpr::new(ColumnRef::new(
-                                TableRef::None,
+                                TableRef::from_names(None, ""),
                                 ident_b.clone(),
                                 ColumnType::BigInt,
                             )))),
@@ -1216,12 +1216,12 @@ mod tests {
                     expr: DynProofExpr::Add(
                         AddExpr::try_new(
                             Box::new(DynProofExpr::Column(ColumnExpr::new(ColumnRef::new(
-                                TableRef::None,
+                                TableRef::from_names(None, ""),
                                 ident_a.clone(),
                                 ColumnType::BigInt,
                             )))),
                             Box::new(DynProofExpr::Column(ColumnExpr::new(ColumnRef::new(
-                                TableRef::None,
+                                TableRef::from_names(None, ""),
                                 ident_b.clone(),
                                 ColumnType::BigInt,
                             )))),