Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
fbe3eb7
feat: throws error for string into integer col
luizotavio32 Aug 12, 2025
0f9d3db
test: throws error for string into integer col
luizotavio32 Aug 12, 2025
5f9188d
test: add float validation test
luizotavio32 Aug 13, 2025
fd13b85
feat: casting is now done after the dataframe is created for better e…
luizotavio32 Aug 19, 2025
bc73dd7
test: casting is now done after the dataframe is created for better e…
luizotavio32 Aug 19, 2025
d05070b
removing unused csv
luizotavio32 Aug 19, 2025
1cf7923
feat: validation for empty table names when Trino is selected
luizotavio32 Aug 20, 2025
75586ef
test: validation for empty table names when Trino is selected
luizotavio32 Aug 20, 2025
79920f8
Merge branch 'master' into upload-csv-fixes
luizotavio32 Aug 20, 2025
142965a
chore: pre-commit idents
luizotavio32 Aug 20, 2025
7cca960
refactor: creates a interface to the selected dataase storing id and …
luizotavio32 Aug 21, 2025
bd40677
feat: new type of excpetion raise due to some code overrides implemet…
luizotavio32 Aug 22, 2025
742849a
revert validation on frontend
luizotavio32 Aug 25, 2025
2dbf71d
revert table name frontend validation
luizotavio32 Aug 25, 2025
c538717
refactor: generic exception handling to acquire possible exception me…
luizotavio32 Aug 28, 2025
6fb05f7
korbit suggestions
luizotavio32 Sep 1, 2025
f3e13b6
perfomance increase suggested by claude
luizotavio32 Sep 2, 2025
f7be36f
checks table name entirely on lower case due to Trino enforcing lower…
luizotavio32 Sep 3, 2025
6215ac0
refactor: compares the entire broken column to check for possible add…
luizotavio32 Sep 5, 2025
05d0a44
fixes pylint warnings
luizotavio32 Sep 5, 2025
6c6df38
Defining error limit to 5 as a constant inside csv_reader
luizotavio32 Sep 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion superset/commands/database/uploaders/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ def _dataframe_to_database(
)
) from ex
except Exception as ex:
raise DatabaseUploadFailed(exception=ex) from ex
message = ex.message if hasattr(ex, "message") and ex.message else str(ex)
raise DatabaseUploadFailed(message=message, exception=ex) from ex

@luizotavio32 luizotavio32 Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handling the exception here will help us catch errors raised inside of databases code overrides df_to_sql() method. It will return a more actionable error rather than Database upload failed



class UploadCommand(BaseCommand):
Expand Down
222 changes: 216 additions & 6 deletions superset/commands/database/uploaders/csv_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@

logger = logging.getLogger(__name__)

# Fixed error limit to avoid huge payloads and poor UX given that a file
# might contain thousands of errors.
MAX_DISPLAYED_ERRORS = 5
Comment on lines +36 to +38

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration compatibility break

The change from configurable CSV_UPLOAD_MAX_ERRORS_DISPLAYED to hardcoded MAX_DISPLAYED_ERRORS = 5 breaks backward compatibility for deployments that have customized this limit. This affects the CSVReader._get_error_details() and CSVReader._create_error_message() methods which use this value to limit error display. Restore the configurable behavior while keeping the improved constant name.

Code suggestion
Check the AI-generated fix before applying
Suggested change
# Fixed error limit to avoid huge payloads and poor UX given that a file
# might contain thousands of errors.
MAX_DISPLAYED_ERRORS = 5
# Fixed error limit to avoid huge payloads and poor UX given that a file
# might contain thousands of errors.
MAX_DISPLAYED_ERRORS = current_app.config.get("CSV_UPLOAD_MAX_ERRORS_DISPLAYED", 5)

Code Review Run #efe81c


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


ROWS_TO_READ_METADATA = 100
DEFAULT_ENCODING = "utf-8"
ENCODING_FALLBACKS = ["utf-8", "latin-1", "cp1252", "iso-8859-1"]
Expand Down Expand Up @@ -123,6 +127,205 @@ def _select_optimal_engine() -> str:
)
return "c"

@staticmethod
def _find_invalid_values_numeric(df: pd.DataFrame, column: str) -> pd.Series:
"""
Find invalid values for numeric type conversion.

Identifies rows where values cannot be converted to numeric types using
pandas to_numeric with error coercing. Returns a boolean mask indicating
which values are invalid (non-null but unconvertible).

:param df: DataFrame containing the data
:param column: Name of the column to check for invalid values

:return: Boolean Series indicating which rows have invalid
values for numeric conversion
"""
converted = pd.to_numeric(df[column], errors="coerce")
return converted.isna() & df[column].notna()

@staticmethod
def _find_invalid_values_non_numeric(
df: pd.DataFrame, column: str, dtype: str
) -> pd.Series:
"""
Find invalid values for non-numeric type conversion.

Identifies rows where values cannot be converted to the specified non-numeric
data type by attempting conversion and catching exceptions. This is used for
string, categorical, or other non-numeric type conversions.

:param df: DataFrame containing the data
:param column: Name of the column to check for invalid values
:param dtype: Target data type for conversion (e.g., 'string', 'category')

:return: Boolean Series indicating which rows have
invalid values for the target type
"""
invalid_mask = pd.Series([False] * len(df), index=df.index)
for idx, value in df[column].items():
if pd.notna(value):
try:
pd.Series([value]).astype(dtype)
except (ValueError, TypeError):
invalid_mask[idx] = True
return invalid_mask

@staticmethod
def _get_error_details(
df: pd.DataFrame,
column: str,
dtype: str,
invalid_mask: pd.Series,
kwargs: dict[str, Any],
) -> tuple[list[str], int]:
"""
Get detailed error information for invalid values in type conversion.

Extracts detailed information about conversion errors, including specific
invalid values and their line numbers. Limits the number of detailed errors
shown to avoid overwhelming output while providing total error count.

:param df: DataFrame containing the data
:param column: Name of the column with conversion errors
:param dtype: Target data type that failed conversion
:param invalid_mask: Boolean mask indicating which rows have invalid values
:param kwargs: Additional parameters including header row information

:return: Tuple containing:
- List of formatted error detail strings (limited by MAX_DISPLAYED_ERRORS)
- Total count of errors found
"""
if not invalid_mask.any():
return [], 0

invalid_indices = invalid_mask[invalid_mask].index.tolist()
total_errors = len(invalid_indices)

error_details = []
for idx in invalid_indices[:MAX_DISPLAYED_ERRORS]:
invalid_value = df.loc[idx, column]
line_number = idx + kwargs.get("header", 0) + 2
error_details.append(
f" • Line {line_number}: '{invalid_value}' cannot be converted to "
f"{dtype}"
)

return error_details, total_errors

@staticmethod
def _create_error_message(
df: pd.DataFrame,
column: str,
dtype: str,
invalid_mask: pd.Series,
kwargs: dict[str, Any],
original_error: Exception,
) -> str:
"""
Create detailed error message for type conversion failure.

Constructs a comprehensive error message that includes:
- Column name and target type
- Total count of errors found
- Detailed list of first few errors with line numbers and values
- Summary of remaining errors if exceeding display limit

:param df: DataFrame containing the data
:param column: Name of the column that failed conversion
:param dtype: Target data type that failed
:param invalid_mask: Boolean mask indicating which rows have invalid values
:param kwargs: Additional parameters including header information
:param original_error: Original exception that triggered the error handling

:return: Formatted error message string ready for display to user
"""
error_details, total_errors = CSVReader._get_error_details(
df, column, dtype, invalid_mask, kwargs
)

if error_details:
base_msg = (
f"Cannot convert column '{column}' to {dtype}. "
f"Found {total_errors} error(s):"
)
detailed_errors = "\n".join(error_details)

if total_errors > MAX_DISPLAYED_ERRORS:
remaining = total_errors - MAX_DISPLAYED_ERRORS
additional_msg = f"\n ... and {remaining} more error(s)"
return f"{base_msg}\n{detailed_errors}{additional_msg}"
else:
return f"{base_msg}\n{detailed_errors}"
else:
return f"Cannot convert column '{column}' to {dtype}. {str(original_error)}"

@staticmethod
def _cast_single_column(
df: pd.DataFrame, column: str, dtype: str, kwargs: dict[str, Any]
) -> None:
"""
Cast a single DataFrame column to the specified data type.

Attempts to convert a column to the target data type with enhanced error
handling. For numeric types, uses pandas to_numeric for better performance
and error detection. If conversion fails, provides detailed
error messages including specific invalid values and their line numbers.

:param df: DataFrame to modify (modified in-place)
:param column: Name of the column to cast
:param dtype: Target data type (e.g., 'int64', 'float64', 'string')
:param kwargs: Additional parameters including header row information

:raises DatabaseUploadFailed: If type conversion fails,
with detailed error message
"""
numeric_types = {"int64", "int32", "float64", "float32"}

try:
if dtype in numeric_types:
df[column] = pd.to_numeric(df[column], errors="raise")
df[column] = df[column].astype(dtype)
else:
df[column] = df[column].astype(dtype)
except (ValueError, TypeError) as ex:
try:
if dtype in numeric_types:
invalid_mask = CSVReader._find_invalid_values_numeric(df, column)
else:
invalid_mask = CSVReader._find_invalid_values_non_numeric(
df, column, dtype
)

error_msg = CSVReader._create_error_message(
df, column, dtype, invalid_mask, kwargs, ex
)
except Exception:
error_msg = f"Cannot convert column '{column}' to {dtype}. {str(ex)}"

raise DatabaseUploadFailed(message=error_msg) from ex

@staticmethod
def _cast_column_types(
df: pd.DataFrame, types: dict[str, str], kwargs: dict[str, Any]
) -> pd.DataFrame:
"""
Cast DataFrame columns to specified types with detailed
error reporting.

:param df: DataFrame to cast
:param types: Dictionary mapping column names to target types
:param kwargs: Original read_csv kwargs for line number calculation
:return: DataFrame with casted columns
:raises DatabaseUploadFailed: If type conversion fails with detailed error info
"""
for column, dtype in types.items():
if column not in df.columns:
continue
CSVReader._cast_single_column(df, column, dtype, kwargs)
return df

@staticmethod
def _read_csv( # noqa: C901
file: FileStorage,
Expand Down Expand Up @@ -154,6 +357,7 @@ def _read_csv( # noqa: C901
kwargs["low_memory"] = False

try:
types = kwargs.pop("dtype", None)
if "chunksize" in kwargs:
chunks = []
total_rows = 0
Expand Down Expand Up @@ -188,13 +392,19 @@ def _read_csv( # noqa: C901
index_col = kwargs.get("index_col")
if isinstance(index_col, str):
result.index.name = index_col
return result
return pd.DataFrame()
df = result
else:
df = pd.read_csv(
filepath_or_buffer=file.stream,
**kwargs,
)

return pd.read_csv(
filepath_or_buffer=file.stream,
**kwargs,
)
if types:
df = CSVReader._cast_column_types(df, types, kwargs)

return df
except DatabaseUploadFailed:
raise
except UnicodeDecodeError as ex:
if encoding != DEFAULT_ENCODING:
raise DatabaseUploadFailed(
Expand Down
4 changes: 3 additions & 1 deletion superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,9 @@ def get_perm(self) -> str:
def has_table(self, table: Table) -> bool:
with self.get_sqla_engine(catalog=table.catalog, schema=table.schema) as engine:
# do not pass "" as an empty schema; force null
return engine.has_table(table.table, table.schema or None)
if engine.has_table(table.table, table.schema or None):
return True
return engine.has_table(table.table.lower(), table.schema or None)

def has_view(self, table: Table) -> bool:
with self.get_sqla_engine(catalog=table.catalog, schema=table.schema) as engine:
Expand Down
Loading
Loading