Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion ddpui/api/charts_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ def get_chart_data(request, payload: ChartDataPayload):
f"Chart data request - Type: {payload.computation_type}, Schema: {payload.schema_name}, Table: {payload.table_name}"
)
logger.info(
f"Columns - x_axis: {payload.x_axis}, y_axis: {payload.y_axis}, dimension_col: {payload.dimension_col}, metrics: {payload.metrics}"
f"Columns - x_axis: {payload.x_axis}, y_axis: {payload.y_axis}, dimension_col: {payload.dimension_col}, extra_dimension: {payload.extra_dimension}, metrics: {payload.metrics}"
)

# Validate user has access to schema/table
Expand Down
242 changes: 180 additions & 62 deletions ddpui/core/charts/charts_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,18 @@ def build_chart_query(
query_builder.fetch_from(payload.table_name, payload.schema_name)

if payload.computation_type == "raw":
# Collect unique columns to avoid duplicates
columns_to_add = set()
if payload.x_axis:
query_builder.add_column(column(payload.x_axis))
columns_to_add.add(payload.x_axis)
if payload.y_axis:
query_builder.add_column(column(payload.y_axis))
columns_to_add.add(payload.y_axis)
if payload.extra_dimension:
query_builder.add_column(column(payload.extra_dimension))
columns_to_add.add(payload.extra_dimension)

# Add each unique column to the query
for col_name in columns_to_add:
query_builder.add_column(column(col_name))

# Validate that at least one column is specified
if not payload.x_axis and not payload.y_axis:
Expand Down Expand Up @@ -549,16 +555,26 @@ def execute_chart_query(
column_mapping = []

if payload.computation_type == "raw":
# For raw queries, columns are in the order they were added
col_index = 0
# For raw queries, build mapping based on unique columns
# Collect unique columns in the same order as in build_query
columns_to_add = set()
column_order = [] # Preserve insertion order
if payload.x_axis:
column_mapping.append((payload.x_axis, col_index))
col_index += 1
if payload.x_axis not in columns_to_add:
columns_to_add.add(payload.x_axis)
column_order.append(payload.x_axis)
if payload.y_axis:
column_mapping.append((payload.y_axis, col_index))
col_index += 1
if payload.y_axis not in columns_to_add:
columns_to_add.add(payload.y_axis)
column_order.append(payload.y_axis)
if payload.extra_dimension:
column_mapping.append((payload.extra_dimension, col_index))
if payload.extra_dimension not in columns_to_add:
columns_to_add.add(payload.extra_dimension)
column_order.append(payload.extra_dimension)

# Build mapping using the unique column order
for col_index, col_name in enumerate(column_order):
column_mapping.append((col_name, col_index))

else: # aggregated
col_index = 0
Expand Down Expand Up @@ -604,24 +620,63 @@ def transform_data_for_chart(

if payload.chart_type == "bar":
if payload.computation_type == "raw":
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
# Handle extra dimension for grouping/stacking in raw charts
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()

for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)

x_values.add(x_val)

if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val

# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []

for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))

return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label),
preserve_none=True,
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
Comment on lines 621 to +679
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: y-axis nulls become "Unknown" strings in raw bar.

Using safe_get_value on numeric y injects the null label into series data. Use row.get(...) and convert_value(..., preserve_none=True) to keep None numeric-safe.

-                    y_val = convert_value(
-                        safe_get_value(row, payload.y_axis, null_label), preserve_none=True
-                    )
+                    y_val = convert_value(row.get(payload.y_axis), preserve_none=True)

Also consider retaining original x order instead of sorted() if the dataset order is meaningful.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if payload.chart_type == "bar":
if payload.computation_type == "raw":
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
# Handle extra dimension for grouping/stacking in raw charts
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()
for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)
x_values.add(x_val)
if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val
# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []
for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))
return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label),
preserve_none=True,
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
if payload.chart_type == "bar":
if payload.computation_type == "raw":
# Handle extra dimension for grouping/stacking in raw charts
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()
for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(row.get(payload.y_axis), preserve_none=True)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)
x_values.add(x_val)
if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val
# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []
for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))
return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label),
preserve_none=True,
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
🤖 Prompt for AI Agents
In ddpui/core/charts/charts_service.py around lines 621 to 679, fix the raw
bar-chart branch so numeric y nulls are preserved as None (not replaced by the
null label) and original x order is retained when extra_dimension is present:
when extracting y values use row.get(payload.y_axis, None) (not safe_get_value)
and pass that to convert_value(..., preserve_none=True); for x values, preserve
input order instead of calling sorted() by collecting x values into a list while
tracking a seen set (append new x when first encountered) and use that ordered
list for xAxisData and for building series with data_dict.get(x_val, 0).

else: # aggregated
if not payload.metrics or len(payload.metrics) == 0:
return {}
Expand Down Expand Up @@ -735,15 +790,33 @@ def transform_data_for_chart(

elif payload.chart_type == "pie":
if payload.computation_type == "raw":
# For raw data, count occurrences
value_counts = {}
for row in results:
key = handle_null_value(safe_get_value(row, payload.x_axis, null_label), null_label)
value_counts[key] = value_counts.get(key, 0) + 1
# For truly raw data, each record becomes a separate pie slice
pie_data = []
for i, row in enumerate(results):
x_value = handle_null_value(
safe_get_value(row, payload.x_axis, null_label), null_label
)

if payload.y_axis:
# Use Y-axis value as the slice value
y_value = convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
slice_value = y_value if y_value is not None else 0
# Create unique slice name combining X and Y values
slice_name = f"{x_value} ({slice_value})"
else:
# If no Y-axis, each record gets value of 1
slice_value = 1
# Create unique slice name with index to avoid duplicates
slice_name = f"{x_value} #{i+1}"

pie_data.append({"value": slice_value, "name": slice_name})

series_name = payload.y_axis if payload.y_axis else f"Records by {payload.x_axis}"
return {
"pieData": [{"value": count, "name": name} for name, count in value_counts.items()],
"seriesName": payload.x_axis,
"pieData": pie_data,
"seriesName": series_name,
}
Comment on lines 791 to 820
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: pie slice values can become non-numeric "Unknown".

For raw pie with y_axis, safe_get_value converts None to a string label, breaking ECharts numeric expectations. Fetch raw and preserve None.

-                    y_value = convert_value(
-                        safe_get_value(row, payload.y_axis, null_label), preserve_none=True
-                    )
+                    y_value = convert_value(row.get(payload.y_axis), preserve_none=True)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In ddpui/core/charts/charts_service.py around lines 791 to 820, the raw pie
branch currently converts None Y values into the null label string which
produces non-numeric slice values; change the Y-axis value retrieval to fetch
the raw value while preserving None (i.e., call safe_get_value for
payload.y_axis with preserve_none=True or otherwise avoid mapping None to
null_label), then pass that raw value into convert_value with preserve_none=True
so that None stays None and you can set slice_value = y_value if y_value is not
None else 0; keep slice_name logic but only include the numeric y_value when it
is not None to avoid embedding the null label into the numeric field.

else: # aggregated
if not payload.metrics or len(payload.metrics) == 0:
Expand Down Expand Up @@ -790,24 +863,61 @@ def transform_data_for_chart(

elif payload.chart_type == "line":
if payload.computation_type == "raw":
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
# Check if we have extra_dimension for grouping
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()
for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)

x_values.add(x_val)
if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val

# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []

for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))

return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label),
preserve_none=True,
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
else: # aggregated
Comment on lines 865 to 921
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Bug: y-axis null handling in raw line mirrors bar issue.

Apply the same fix to avoid "Unknown" in numeric series.

-                    y_val = convert_value(
-                        safe_get_value(row, payload.y_axis, null_label), preserve_none=True
-                    )
+                    y_val = convert_value(row.get(payload.y_axis), preserve_none=True)
-                            "data": [
-                                convert_value(
-                                    safe_get_value(row, payload.y_axis, null_label),
-                                    preserve_none=True,
-                                )
-                                for row in results
-                            ],
+                            "data": [
+                                convert_value(row.get(payload.y_axis), preserve_none=True)
+                                for row in results
+                            ],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if payload.computation_type == "raw":
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
# Check if we have extra_dimension for grouping
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()
for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(
safe_get_value(row, payload.y_axis, null_label), preserve_none=True
)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)
x_values.add(x_val)
if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val
# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []
for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))
return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(
safe_get_value(row, payload.y_axis, null_label),
preserve_none=True,
)
for row in results
],
}
],
"legend": [payload.y_axis],
}
else: # aggregated
if payload.computation_type == "raw":
# Check if we have extra_dimension for grouping
if payload.extra_dimension:
# Group data by extra dimension
grouped_data = {}
x_values = set()
for row in results:
x_val = convert_value(safe_get_value(row, payload.x_axis, null_label))
y_val = convert_value(row.get(payload.y_axis), preserve_none=True)
extra_val = convert_value(
safe_get_value(row, payload.extra_dimension, null_label)
)
x_values.add(x_val)
if extra_val not in grouped_data:
grouped_data[extra_val] = {}
grouped_data[extra_val][x_val] = y_val
# Build series data for each extra dimension value
x_axis_data = sorted(list(x_values))
series_data = []
legend_data = []
for extra_val, data_dict in grouped_data.items():
series_values = [data_dict.get(x_val, 0) for x_val in x_axis_data]
series_data.append({"name": str(extra_val), "data": series_values})
legend_data.append(str(extra_val))
return {
"xAxisData": x_axis_data,
"series": series_data,
"legend": legend_data,
}
else:
# Simple raw chart without extra dimension
return {
"xAxisData": [
convert_value(safe_get_value(row, payload.x_axis, null_label))
for row in results
],
"series": [
{
"name": payload.y_axis,
"data": [
convert_value(row.get(payload.y_axis), preserve_none=True)
for row in results
],
}
],
"legend": [payload.y_axis],
}
else: # aggregated
🤖 Prompt for AI Agents
In ddpui/core/charts/charts_service.py around lines 865-921, the grouped
raw-path currently fills missing x-axis points with 0 (series_values =
[data_dict.get(x_val, 0) ...]) which causes numeric series to show
"Unknown"/incorrect bars; change the default to None so missing points remain
nulls (series_values = [data_dict.get(x_val, None) ...]) and ensure the grouped
y values are converted with preserve_none=True (leave that conversion in place)
so charts receive None for missing data instead of 0 or the string "Unknown".

if not payload.metrics or len(payload.metrics) == 0:
return {}
Expand Down Expand Up @@ -1011,18 +1121,26 @@ def get_chart_data_table_preview(
columns = []

if payload.computation_type == "raw":
col_index = 0
# For raw queries, build mapping based on unique columns (same logic as in execute_chart_query)
columns_to_add = set()
column_order = [] # Preserve insertion order
if payload.x_axis:
column_mapping.append((payload.x_axis, col_index))
columns.append(payload.x_axis)
col_index += 1
if payload.x_axis not in columns_to_add:
columns_to_add.add(payload.x_axis)
column_order.append(payload.x_axis)
if payload.y_axis:
column_mapping.append((payload.y_axis, col_index))
columns.append(payload.y_axis)
col_index += 1
if payload.y_axis not in columns_to_add:
columns_to_add.add(payload.y_axis)
column_order.append(payload.y_axis)
if payload.extra_dimension:
column_mapping.append((payload.extra_dimension, col_index))
columns.append(payload.extra_dimension)
if payload.extra_dimension not in columns_to_add:
columns_to_add.add(payload.extra_dimension)
column_order.append(payload.extra_dimension)

# Build mapping using the unique column order
for col_index, col_name in enumerate(column_order):
column_mapping.append((col_name, col_index))
columns.append(col_name)
else: # aggregated
col_index = 0
column_mapping.append((payload.dimension_col, col_index))
Expand Down
Loading