Spaces:
Running
Running
fix: restore type coercion + DDL fallback for insert_rows
Browse filesPrior commit broke coercion by moving the if/elif into wrong scope —
varchar truncation and numeric coercion only ran in the elif branch,
leaving the normal column_info path doing nothing. Restructured so
both column_info and ddl_types paths resolve info/data_type first,
then shared coercion code runs for both.
Also adds is_fact_table name-based detection so schemas without FK
constraints still resolve correctly (SALES, PERFORMANCE, JOURNEYS, etc).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- legitdata_bridge.py +37 -33
- legitdata_project/legitdata/ddl/models.py +36 -5
legitdata_bridge.py
CHANGED
|
@@ -233,7 +233,9 @@ class KeyPairSnowflakeWriter:
|
|
| 233 |
if isinstance(value, date):
|
| 234 |
return value.strftime('%Y-%m-%d')
|
| 235 |
|
| 236 |
-
# Apply constraints based on column metadata
|
|
|
|
|
|
|
| 237 |
col_upper = col_name.upper()
|
| 238 |
if col_upper in column_info:
|
| 239 |
info = column_info[col_upper]
|
|
@@ -242,38 +244,40 @@ class KeyPairSnowflakeWriter:
|
|
| 242 |
# Fallback: use DDL-declared type when DESCRIBE TABLE didn't return this column
|
| 243 |
info = {}
|
| 244 |
data_type = ddl_types[col_upper].upper()
|
| 245 |
-
|
| 246 |
-
#
|
| 247 |
-
|
| 248 |
-
|
| 249 |
-
|
| 250 |
-
|
| 251 |
-
|
| 252 |
-
|
| 253 |
-
|
| 254 |
-
|
| 255 |
-
|
| 256 |
-
|
| 257 |
-
|
| 258 |
-
|
| 259 |
-
|
| 260 |
-
|
| 261 |
-
|
| 262 |
-
|
| 263 |
-
|
| 264 |
-
|
| 265 |
-
|
| 266 |
-
|
| 267 |
-
|
| 268 |
-
|
| 269 |
-
|
| 270 |
-
|
| 271 |
-
|
| 272 |
-
|
| 273 |
-
|
| 274 |
-
|
| 275 |
-
|
| 276 |
-
|
|
|
|
|
|
|
| 277 |
return value
|
| 278 |
|
| 279 |
total_inserted = 0
|
|
|
|
| 233 |
if isinstance(value, date):
|
| 234 |
return value.strftime('%Y-%m-%d')
|
| 235 |
|
| 236 |
+
# Apply constraints based on column metadata.
|
| 237 |
+
# Resolve info/data_type from column_info (DESCRIBE TABLE) first,
|
| 238 |
+
# then fall back to ddl_types (parsed DDL) so coercion always runs.
|
| 239 |
col_upper = col_name.upper()
|
| 240 |
if col_upper in column_info:
|
| 241 |
info = column_info[col_upper]
|
|
|
|
| 244 |
# Fallback: use DDL-declared type when DESCRIBE TABLE didn't return this column
|
| 245 |
info = {}
|
| 246 |
data_type = ddl_types[col_upper].upper()
|
| 247 |
+
else:
|
| 248 |
+
return value # No type info — pass through unchanged
|
| 249 |
+
|
| 250 |
+
# Truncate strings to fit VARCHAR length
|
| 251 |
+
if 'VARCHAR' in data_type or 'TEXT' in data_type:
|
| 252 |
+
max_len = info.get('length', 255)
|
| 253 |
+
if isinstance(value, str) and len(value) > max_len:
|
| 254 |
+
value = value[:max_len]
|
| 255 |
+
|
| 256 |
+
# Clamp numbers to fit DECIMAL precision.
|
| 257 |
+
# Includes Snowflake's internal DESCRIBE TABLE names: FIXED (integers), REAL (floats).
|
| 258 |
+
elif any(t in data_type for t in (
|
| 259 |
+
'NUMBER', 'DECIMAL', 'NUMERIC', 'INT', 'FLOAT',
|
| 260 |
+
'FIXED', 'REAL', 'DOUBLE', 'MONEY',
|
| 261 |
+
)):
|
| 262 |
+
# If a string landed in a numeric column, strip formatting and coerce
|
| 263 |
+
if isinstance(value, str):
|
| 264 |
+
import re as _re
|
| 265 |
+
cleaned = _re.sub(r'[^\d.\-+eE]', '', value.replace(',', ''))
|
| 266 |
+
try:
|
| 267 |
+
value = float(cleaned) if cleaned else None
|
| 268 |
+
except (ValueError, TypeError):
|
| 269 |
+
return None # Can't coerce — use NULL rather than crash
|
| 270 |
+
precision = info.get('precision', 38)
|
| 271 |
+
scale = info.get('scale', 0)
|
| 272 |
+
if isinstance(value, (int, float)):
|
| 273 |
+
# Max value for given precision/scale
|
| 274 |
+
max_val = 10 ** (precision - scale) - (10 ** -scale)
|
| 275 |
+
min_val = -max_val
|
| 276 |
+
value = max(min_val, min(max_val, value))
|
| 277 |
+
# Round to scale
|
| 278 |
+
if scale > 0:
|
| 279 |
+
value = round(value, scale)
|
| 280 |
+
|
| 281 |
return value
|
| 282 |
|
| 283 |
total_inserted = 0
|
legitdata_project/legitdata/ddl/models.py
CHANGED
|
@@ -4,6 +4,28 @@ from dataclasses import dataclass, field
|
|
| 4 |
from enum import Enum
|
| 5 |
from typing import Optional
|
| 6 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 7 |
|
| 8 |
class ColumnClassification(Enum):
|
| 9 |
"""How a column's data should be sourced."""
|
|
@@ -51,13 +73,22 @@ class Table:
|
|
| 51 |
|
| 52 |
@property
|
| 53 |
def is_fact_table(self) -> bool:
|
| 54 |
-
"""
|
| 55 |
-
|
| 56 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 57 |
@property
|
| 58 |
def is_dimension_table(self) -> bool:
|
| 59 |
-
"""
|
| 60 |
-
return
|
| 61 |
|
| 62 |
def get_column(self, name: str) -> Optional[Column]:
|
| 63 |
"""Get a column by name (case-insensitive)."""
|
|
|
|
| 4 |
from enum import Enum
|
| 5 |
from typing import Optional
|
| 6 |
|
| 7 |
+
# Name fragments that strongly suggest a fact/transaction table.
|
| 8 |
+
# Checked against individual words in the table name (split on underscore).
|
| 9 |
+
_FACT_TABLE_KEYWORDS = {
|
| 10 |
+
'SALES', 'FACT', 'TRANSACTION', 'TRANSACTIONS',
|
| 11 |
+
'ORDER', 'ORDERS', 'EVENT', 'EVENTS',
|
| 12 |
+
'METRIC', 'METRICS', 'PERFORMANCE',
|
| 13 |
+
'ACTIVITY', 'ACTIVITIES',
|
| 14 |
+
'BOOKING', 'BOOKINGS', 'RESERVATION', 'RESERVATIONS',
|
| 15 |
+
'REVENUE', 'INVOICE', 'INVOICES',
|
| 16 |
+
'CLAIM', 'CLAIMS', 'TRADE', 'TRADES',
|
| 17 |
+
'DEAL', 'DEALS', 'PIPELINE',
|
| 18 |
+
'JOURNEY', 'JOURNEYS',
|
| 19 |
+
'INTERACTION', 'INTERACTIONS',
|
| 20 |
+
'PURCHASE', 'PURCHASES',
|
| 21 |
+
'PAYMENT', 'PAYMENTS',
|
| 22 |
+
'SESSION', 'SESSIONS',
|
| 23 |
+
'CONVERSION', 'CONVERSIONS',
|
| 24 |
+
'IMPRESSION', 'IMPRESSIONS',
|
| 25 |
+
'SHIPMENT', 'SHIPMENTS',
|
| 26 |
+
'RETURN', 'RETURNS',
|
| 27 |
+
}
|
| 28 |
+
|
| 29 |
|
| 30 |
class ColumnClassification(Enum):
|
| 31 |
"""How a column's data should be sourced."""
|
|
|
|
| 73 |
|
| 74 |
@property
|
| 75 |
def is_fact_table(self) -> bool:
|
| 76 |
+
"""Identify fact tables by FK count and/or name patterns.
|
| 77 |
+
|
| 78 |
+
Primary signal: 2+ foreign keys (schema has explicit FK constraints).
|
| 79 |
+
Secondary signal: table name contains a known fact-table keyword.
|
| 80 |
+
Both paths are checked so AI-generated DDL without FK constraints
|
| 81 |
+
still resolves correctly for standard fact-table naming conventions.
|
| 82 |
+
"""
|
| 83 |
+
if len(self.foreign_keys) >= 2:
|
| 84 |
+
return True
|
| 85 |
+
words = set(self.name.upper().split('_'))
|
| 86 |
+
return bool(words & _FACT_TABLE_KEYWORDS)
|
| 87 |
+
|
| 88 |
@property
|
| 89 |
def is_dimension_table(self) -> bool:
|
| 90 |
+
"""Dimension tables are tables that are not fact tables."""
|
| 91 |
+
return not self.is_fact_table
|
| 92 |
|
| 93 |
def get_column(self, name: str) -> Optional[Column]:
|
| 94 |
"""Get a column by name (case-insensitive)."""
|