Improve metadata error message to identify the error

pydough/errors/error_utils.py

@@ -27,11 +27,15 @@
     "is_positive_int",
     "is_string",
     "is_valid_name",
+    "is_valid_sql_name",
     "simple_join_keys_predicate",
     "unique_properties_predicate",
 ]
 
 
+import builtins
+import keyword
+import re
 from abc import ABC, abstractmethod
 
 import numpy as np
@@ -97,11 +101,189 @@ class ValidName(PyDoughPredicate):
     as the name of a PyDough graph/collection/property.
     """
 
+    def __init__(self):
+        self.error_messages: dict[str, str] = {
+            "identifier": "must be a string that is a valid Python identifier",
+            "python_keyword": "must be a string that is not a Python reserved word or built-in name",
+            "pydough_keyword": "must be a string that is not a PyDough reserved word",
+            "sql_keyword": "must be a string that is not a SQL reserved word",
+        }
+
+    def _error_code(self, obj: object) -> str | None:
+        """Return an error code if invalid, or None if valid."""
+        ret_val: str | None = None
+        # Check that obj is a string
+        if isinstance(obj, str):
+            # Check that obj is a valid Python identifier
+            if not obj.isidentifier():
+                ret_val = "identifier"
+            # Check that obj is not a Python reserved word or built-in name
+            elif keyword.iskeyword(obj) or hasattr(builtins, obj):
+                ret_val = "python_keyword"
+            # Check that obj is not a PyDough reserved word
+            elif self._is_pydough_keyword(obj):
+                ret_val = "pydough_keyword"
+        else:
+            ret_val = "identifier"
+
+        return ret_val
+
+    def _is_pydough_keyword(self, name: str) -> bool:
+        """
+        helper: Verifies if name is a PyDough reserved word.
+                Extend with new PyDough reserved words if required.
+        """
+        # Dictionary of all registered operators pre-built from the PyDough source
+        from pydough.pydough_operators import builtin_registered_operators
+
+        # Set of collection operators
+        PYDOUGH_RESERVED: set[str] = {
+            "CALCULATE",
+            "WHERE",
+            "ORDER_BY",
+            "TOP_K",
+            "PARTITION",
+            "SINGULAR",
+            "BEST",
+            "CROSS",
+        }
+        return (name in PYDOUGH_RESERVED) or (name in builtin_registered_operators())
+
     def accept(self, obj: object) -> bool:
-        return isinstance(obj, str) and obj.isidentifier()
+        return self._error_code(obj) is None
 
     def error_message(self, error_name: str) -> str:
-        return f"{error_name} must be a string that is a Python identifier"
+        # Generic fallback (since we don't have the object here)
+        return f"{error_name} must be a valid identifier and not a reserved word"
+
+    def verify(self, obj: object, error_name: str) -> None:
+        code: str | None = self._error_code(obj)
+        if code is not None:
+            raise PyDoughMetadataException(f"{error_name} {self.error_messages[code]}")
+
+
+class ValidSQLName(PyDoughPredicate):
+    """Predicate class to check that an object is a string that can be used
+    as the name for a SQL table path/column name.
+    """
+
+    # Regex for unquoted SQL identifiers
+    _UNQUOTED_SQL_IDENTIFIER = re.compile(
+        r"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)*$"
+    )
+
+    def __init__(self):
+        self.error_messages: dict[str, str] = {
+            "identifier": "must have a SQL name that is a valid SQL identifier",
+            "sql_keyword": "must have a SQL name that is not a reserved word",
+        }
+
+    def _error_code(self, obj: object) -> str | None:
+        """Return an error code if invalid, or None if valid."""
+        ret_val: str | None = None
+        # Check that obj is a string
+        if isinstance(obj, str):
+            # Check that obj is a valid SQL identifier
+            if not self.is_valid_sql_identifier(obj):
+                ret_val = "identifier"
+            # Check that obj is not a SQL reserved word
+            elif self._is_sql_keyword(obj):
+                ret_val = "sql_keyword"
+        else:
+            ret_val = "identifier"
+
+        return ret_val
+
+    def is_valid_sql_identifier(self, name: str) -> bool:
+        """
+        Check if a string is a valid SQL identifier.
+
+        - Unquoted: starts with letter/underscore, then letters, digits,
+            underscores.
+        - Double-quoted: allows any chars, but " "" " is the only valid way to
+            include a double-quote char.
+        - Backtick-quoted: allows any chars, but `` `` `` is the only valid
+            way to include a backtick char.
+        """
+        if not name:
+            return False
+
+        # Case 1: unquoted
+        if self._UNQUOTED_SQL_IDENTIFIER.match(name):
+            return True
+
+        # Case 2: double quoted
+        if name.startswith('"') and name.endswith('"'):
+            inner = name[1:-1]
+            # Any " must be escaped as ""
+            return '"' not in inner.replace('""', "")
+
+        # Case 3: backtick quoted
+        if name.startswith("`") and name.endswith("`"):
+            inner = name[1:-1]
+            # Any ` must be escaped as ``
+            return "`" not in inner.replace("``", "")
+
+        return False
+
+    # fmt: off
+    SQL_RESERVED_KEYWORDS: set[str] = {
+        # Query & DML
+        "select", "from", "where", "group", "having", "distinct", "as", 
+        "join", "inner", "union", "intersect", "except",
+
+        # DDL & schema
+        "create", "alter", "drop", "table", "view", "index", "sequence",
+        "trigger", "schema", "database", "column", "constraint",
+
+        # DML
+        "insert", "update", "delete", "into", "values", "set",
+
+        # Control flow & logical
+        "and", "or", "not", "in", "is", "like", "between", "case", "when",
+        "then", "else", "end", "exists",
+
+        # Transaction & session
+        "begin", "commit", "rollback", "savepoint", "transaction",
+        "lock", "grant", "revoke",
+
+        # Data types
+        "int", "integer", "bigint", "smallint", "decimal", "numeric",
+        "float", "real", "double", "char", "varchar", "text",
+        "timestamp", "boolean", "null",
+
+        # Functions
+        "cast",
+    }
+    """
+    Set of SQL reserved keywords that may cause conflicts when used as table or
+    column names. This list was compiled from commonly reserved terms across
+    multiple SQL dialects (e.g., PostgreSQL, SQLite, MySQL), with emphasis on
+    keywords that are likely to appear in generated SQL statements.
+    If any of these are used as identifiers, they must be properly escaped to
+    avoid syntax errors.
+    """
+    # fmt: on
+
+    def _is_sql_keyword(self, name: str) -> bool:
+        """
+        helper: Verifies if name is a SQL reserved word.
+                Uses SQL_RESERVED_KEYWORDS set.
+                Extend with new SQL reserved words if required.
+        """
+        return name.lower() in self.SQL_RESERVED_KEYWORDS
+
+    def accept(self, obj: object) -> bool:
+        return self._error_code(obj) is None
+
+    def error_message(self, error_name: str) -> str:
+        # Generic fallback (since we don't have the object here)
+        return f"{error_name} must be a valid SQL identifier and not a reserved word"
+
+    def verify(self, obj: object, error_name: str) -> None:
+        code: str | None = self._error_code(obj)
+        if code is not None:
+            raise PyDoughMetadataException(f"{error_name} {self.error_messages[code]}")
 
 
 class NoExtraKeys(PyDoughPredicate):
@@ -304,6 +486,7 @@ def error_message(self, error_name: str) -> str:
 ###############################################################################
 
 is_valid_name: PyDoughPredicate = ValidName()
+is_valid_sql_name: PyDoughPredicate = ValidSQLName()
 is_integer = HasType(int, "integer")
 is_string = HasType(str, "string")
 is_bool = HasType(bool, "boolean")

pydough/metadata/collections/collection_metadata.py

@@ -48,8 +48,8 @@ def __init__(
             PropertyMetadata,
         )
 
-        is_valid_name.verify(name, "name")
-        HasType(GraphMetadata).verify(graph, "graph")
+        is_valid_name.verify(name, f"collection name {name!r}")
+        HasType(GraphMetadata).verify(graph, f"graph {name!r}")
 
         self._graph: GraphMetadata = graph
         self._name: str = name

pydough/metadata/collections/simple_table_metadata.py

@@ -10,6 +10,7 @@
     extract_array,
     extract_string,
     is_string,
+    is_valid_sql_name,
     unique_properties_predicate,
 )
 from pydough.metadata.graphs import GraphMetadata
@@ -146,6 +147,7 @@ def parse_from_json(
         # Extract the relevant properties from the JSON to build the new
         # collection, then add it to the graph.
         table_path: str = extract_string(collection_json, "table path", error_name)
+        is_valid_sql_name.verify(table_path, error_name)
         HasPropertyWith("unique properties", unique_properties_predicate).verify(
             collection_json, error_name
         )
@@ -161,8 +163,6 @@ def parse_from_json(
         )
         # Parse the optional common semantic properties like the description.
         new_collection.parse_optional_properties(collection_json)
-        properties: list = extract_array(
-            collection_json, "properties", new_collection.error_name
-        )
+        properties: list = extract_array(collection_json, "properties", error_name)
         new_collection.add_properties_from_json(properties)
         graph.add_collection(new_collection)

pydough/metadata/graphs/graph_metadata.py

@@ -43,7 +43,7 @@ def __init__(
         synonyms: list[str] | None,
         extra_semantic_info: dict | None,
     ):
-        is_valid_name.verify(name, "graph name")
+        is_valid_name.verify(name, f"graph name {name!r}")
         self._additional_definitions: list[str] | None = additional_definitions
         self._verified_pydough_analysis: list[dict] | None = verified_pydough_analysis
         self._name: str = name
@@ -179,7 +179,7 @@ def add_function(self, name: str, function: "ExpressionFunctionOperator") -> Non
             `PyDoughMetadataException`: if `function` cannot be inserted
             into the graph because of a name collision.
         """
-        is_valid_name.verify(name, "function name")
+        is_valid_name.verify(name, f"function name {name!r}")
         if name == self.name:
             raise PyDoughMetadataException(
                 f"Function name {name!r} cannot be the same as the graph name {self.name!r}"

pydough/metadata/properties/property_metadata.py

@@ -45,8 +45,8 @@ def __init__(
         synonyms: list[str] | None,
         extra_semantic_info: dict | None,
     ):
-        is_valid_name.verify(name, "name")
-        HasType(CollectionMetadata).verify(collection, "collection")
+        is_valid_name.verify(name, f"property name {name!r}")
+        HasType(CollectionMetadata).verify(collection, f"collection {name}")
         self._name: str = name
         self._collection: CollectionMetadata = collection
         super().__init__(description, synonyms, extra_semantic_info)

pydough/metadata/properties/table_column_metadata.py

@@ -11,6 +11,7 @@
     NoExtraKeys,
     extract_string,
     is_string,
+    is_valid_sql_name,
 )
 from pydough.metadata.collections import CollectionMetadata
 from pydough.types import PyDoughType, parse_type_from_string
@@ -99,6 +100,7 @@ def parse_from_json(
         except PyDoughTypeException as e:
             raise PyDoughMetadataException(*e.args)
         column_name: str = extract_string(property_json, "column name", error_name)
+        is_valid_sql_name.verify(column_name, error_name)
 
         NoExtraKeys(TableColumnMetadata.allowed_fields).verify(
             property_json, error_name

tests/test_metadata/defog_graphs.json

@@ -223,7 +223,7 @@
             "synonyms": ["record datetime", "price update date"]
           },
           {
-            "name": "open",
+            "name": "_open",
             "type": "table column",
             "column name": "sbDpOpen",
             "data type": "numeric",