Skip to content

Commit 920e663

Browse files
Merge remote-tracking branch 'origin/develop' into feature/elasticsearch-handler-update
2 parents e9c6269 + 46bc61a commit 920e663

26 files changed

Lines changed: 1495 additions & 366 deletions

.github/workflows/build_deploy_staging.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ on:
1010
types:
1111
- closed
1212
branches:
13-
- 'main'
1413
- 'develop'
1514
- 'release/*'
1615

mindsdb/api/executor/command_executor.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,9 @@ def answer_drop_database(self, statement: DropDatabase | DropDatasource) -> Exec
12861286

12871287
def answer_alter_database(self, statement: AlterDatabase) -> ExecuteAnswer:
12881288
db_name = match_one_part_name(statement.name)
1289-
self.session.database_controller.update(db_name, data=statement.params, strict_case=statement.name.is_quoted[0])
1289+
self.session.database_controller.update(
1290+
db_name, data=statement.params, strict_case=statement.name.is_quoted[0], check_connection=True
1291+
)
12901292
return ExecuteAnswer()
12911293

12921294
def answer_drop_tables(self, statement, database_name):

mindsdb/api/executor/planner/plan_join.py

Lines changed: 197 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
from typing import List
21
import copy
32
from dataclasses import dataclass, field
43

@@ -36,8 +35,8 @@
3635
class TableInfo:
3736
integration: str
3837
table: Identifier
39-
aliases: List[str] = field(default_factory=List)
40-
conditions: List = None
38+
aliases: list[tuple[str, ...]] = field(default_factory=list)
39+
conditions: list = None
4140
sub_select: ast.ASTNode = None
4241
predictor_info: dict = None
4342
join_condition = None
@@ -279,49 +278,27 @@ def _check_node_condition(node, **kwargs):
279278
self.query_context["binary_ops"] = binary_ops
280279

281280
def check_use_limit(self, query_in, join_sequence):
282-
"""
283-
Determine if LIMIT can be pushed down to the first table fetch.
284-
285-
LIMIT pushdown means: fetch only N rows from the first table, then join.
286-
This optimization is ONLY correct when the first table determines the final row count.
287-
288-
LIMIT pushdown is CORRECT for:
289-
- Single table queries (no join)
290-
- LEFT JOIN (left table determines row count - each left row appears exactly once)
291-
- Joins with ML predictors
292-
293-
LIMIT pushdown is SLOW for:
294-
- INNER JOIN between tables
295-
- RIGHT JOIN (right table determines row count, not left)
296-
297-
When LIMIT pushdown is disabled, we fetch all data and apply LIMIT after the join.
298-
This is slower but guarantees correct results.
299-
"""
281+
# if only models (predictors), not for regular table joins
300282
use_limit = False
301283
if query_in.having is None and query_in.group_by is None and query_in.limit is not None:
302284
use_limit = True
303-
304-
# Check what we're joining
305-
has_predictor = False
306-
cannot_pushdown_limit = False
285+
has_join = False
286+
regular_table_count = 0
307287

308288
for item in join_sequence:
309289
if isinstance(item, TableInfo):
310-
if item.predictor_info is not None:
311-
has_predictor = True
290+
# Check if it's a regular table (not a predictor, not a subselect)
291+
if item.predictor_info is None and item.sub_select is None:
292+
regular_table_count += 1
312293
elif isinstance(item, Join):
313-
join_type = (
314-
item.join_type.upper() if hasattr(item.join_type, "upper") else str(item.join_type).upper()
315-
)
316-
317294
# LEFT JOIN preserves left table row count - LIMIT pushdown is safe
318-
if join_type in ("LEFT JOIN", "LEFT OUTER JOIN"):
319-
continue
295+
join_type = str(item.join_type).upper() if item.join_type else ""
296+
if join_type not in ("LEFT JOIN", "LEFT OUTER JOIN"):
297+
has_join = True
320298

321-
# INNER/RIGHT JOIN: can't push LIMIT down
322-
cannot_pushdown_limit = True
323-
324-
if cannot_pushdown_limit and not has_predictor:
299+
# Disable limit pushdown only if joining MULTIPLE regular database tables
300+
# Allow it for: single table, or table + predictor (predictor generates on-demand)
301+
if has_join and regular_table_count > 1:
325302
use_limit = False
326303

327304
self.query_context["use_limit"] = use_limit
@@ -351,6 +328,7 @@ def replace_subselects(node, **args):
351328

352329
# get all join tables, form join sequence
353330
join_sequence = self.get_join_sequence(query.from_table)
331+
self.join_sequence = join_sequence
354332

355333
# find tables for identifiers used in query
356334
def _check_identifiers(node, is_table, **kwargs):
@@ -382,7 +360,7 @@ def _check_identifiers(node, is_table, **kwargs):
382360
for item in join_sequence:
383361
if isinstance(item, TableInfo):
384362
if item.sub_select is not None:
385-
self.process_subselect(item)
363+
self.process_subselect(item, query_in)
386364
elif item.predictor_info is not None:
387365
self.process_predictor(item, query_in)
388366
else:
@@ -409,16 +387,26 @@ def _check_identifiers(node, is_table, **kwargs):
409387
self.close_partition()
410388
return self.step_stack.pop()
411389

412-
def process_subselect(self, item):
390+
def process_subselect(self, item, query_in):
413391
# is sub select
414392
item.sub_select.alias = None
415393
item.sub_select.parentheses = False
416394
step = self.planner.plan_select(item.sub_select)
417395

418396
where = filters_to_bin_op(item.conditions)
419397

398+
# Column pruning for subselects:
399+
# - If subselect has pure SELECT *, we can prune to only needed columns
400+
# - If subselect has explicit columns (SELECT a, b, c), pass through all (don't prune)
401+
# This preserves column aliases and prevents breaking explicit projections
402+
targets = [Star()]
403+
if self._can_prune_columns(item):
404+
needed_columns = self.get_columns_for_table(item, query_in, self.join_sequence)
405+
if needed_columns:
406+
targets = needed_columns
407+
420408
# apply table alias
421-
query2 = Select(targets=[Star()], where=where)
409+
query2 = Select(targets=targets, where=where)
422410
if item.table.alias is None:
423411
raise PlanningException(f"Subselect in join have to be aliased: {item.sub_select.to_string()}")
424412
table_name = item.table.alias.parts[-1]
@@ -431,15 +419,181 @@ def process_subselect(self, item):
431419
step2 = self.add_plan_step(step2)
432420
self.step_stack.append(step2)
433421

422+
def _collect_from_order_by(self, query_in, alias_map, add_column_callback):
423+
"""Helper to collect columns from ORDER BY clause, resolving aliases and ordinals."""
424+
for order_col in query_in.order_by:
425+
field = order_col.field
426+
427+
# Handle ORDER BY ordinal (e.g., ORDER BY 1)
428+
if isinstance(field, Constant) and isinstance(field.value, int):
429+
ordinal = field.value
430+
if 1 <= ordinal <= len(query_in.targets):
431+
target_expr = query_in.targets[ordinal - 1]
432+
query_traversal(target_expr, add_column_callback)
433+
continue
434+
435+
# Handle ORDER BY alias (e.g., ORDER BY alias_name)
436+
if isinstance(field, Identifier) and len(field.parts) == 1:
437+
alias_name = field.parts[0].lower()
438+
if alias_name in alias_map:
439+
query_traversal(alias_map[alias_name], add_column_callback)
440+
continue
441+
442+
# Regular column reference
443+
query_traversal(field, add_column_callback)
444+
445+
def _join_has_predictor(self, join_sequence) -> bool:
446+
"""Check if the join sequence contains any predictor."""
447+
for item in join_sequence:
448+
if isinstance(item, TableInfo) and item.predictor_info is not None:
449+
return True
450+
return False
451+
452+
def _can_prune_columns(self, table_info) -> bool:
453+
"""
454+
Determine if column pruning can be applied to this table.
455+
456+
Returns:
457+
True if column pruning can be applied
458+
False if we should skip pruning (use SELECT *)
459+
"""
460+
# Predictors/models: cannot prune (need all input features)
461+
if table_info.predictor_info is not None:
462+
return False
463+
464+
# If this table is part of a join with a predictor: cannot prune
465+
# Predictors may need all columns from joined tables as input features
466+
if hasattr(self, "join_sequence") and self._join_has_predictor(self.join_sequence):
467+
return False
468+
469+
# For subselects: can only prune if they have pure SELECT * (no other columns)
470+
sub = table_info.sub_select
471+
if sub is not None and isinstance(sub, Select):
472+
targets = getattr(sub, "targets", None) or []
473+
# Can prune only if subselect has PURE SELECT * (one target that is Star)
474+
# Cannot prune if:
475+
# - Mixed: SELECT *, col1 (has Star but also other columns)
476+
if len(targets) == 1 and isinstance(targets[0], Star):
477+
return True # Pure SELECT * - can prune
478+
return False
479+
480+
# For project tables (KB tables, views, etc.): cannot prune
481+
# Project tables need SELECT * for proper column mapping
482+
if table_info.integration and table_info.integration in self.planner.projects:
483+
return False
484+
485+
# Regular integration tables: can prune
486+
return True
487+
488+
def get_columns_for_table(self, table_info, query_in, join_sequence):
489+
"""
490+
Collect all columns needed from a specific table for column pruning optimization.
491+
492+
Note: Caller should check _can_prune_columns() before calling this method.
493+
494+
Returns a list of column Identifiers or None if we should fetch all columns.
495+
"""
496+
columns = {}
497+
has_qualified_star_for_table = False
498+
499+
alias_map = {}
500+
if query_in.targets:
501+
for target in query_in.targets:
502+
if isinstance(target, Identifier) and target.alias:
503+
alias_map[target.alias.parts[-1].lower()] = target
504+
505+
def add_column(node, **kwargs):
506+
if isinstance(node, Identifier):
507+
col_table = self.get_table_for_column(node)
508+
if not col_table or col_table.index != table_info.index:
509+
return
510+
511+
# Check for qualified star: t1.* or alias.*
512+
if node.parts and (node.parts[-1] == "*" or isinstance(node.parts[-1], Star)):
513+
nonlocal has_qualified_star_for_table
514+
has_qualified_star_for_table = True
515+
return
516+
517+
col_name = node.parts[-1]
518+
# Make sure col_name is a string, not a Star object
519+
if not isinstance(col_name, str):
520+
return
521+
522+
is_quoted = node.is_quoted[-1] if node.is_quoted and len(node.is_quoted) == len(node.parts) else False
523+
# Store - if already exists, keep it quoted if either reference was quoted
524+
if col_name in columns:
525+
columns[col_name] = columns[col_name] or is_quoted
526+
else:
527+
columns[col_name] = is_quoted
528+
elif isinstance(node, Function):
529+
# Traverse window function clauses (PARTITION BY, ORDER BY)
530+
if hasattr(node, "partition_by") and node.partition_by:
531+
query_traversal(node.partition_by, add_column)
532+
if hasattr(node, "order_by") and node.order_by:
533+
for order_item in node.order_by:
534+
query_traversal(order_item.field if hasattr(order_item, "field") else order_item, add_column)
535+
536+
# Check for bare Star() in targets
537+
if query_in.targets:
538+
for target in query_in.targets:
539+
if isinstance(target, Star):
540+
return None
541+
542+
# Collect columns from SELECT targets
543+
if query_in.targets:
544+
query_traversal(query_in.targets, add_column)
545+
546+
# Collect columns from WHERE clause
547+
if query_in.where:
548+
query_traversal(query_in.where, add_column)
549+
550+
# Collect columns from ORDER BY (resolve aliases and ordinals)
551+
if query_in.order_by:
552+
self._collect_from_order_by(query_in, alias_map, add_column)
553+
554+
# Collect columns from GROUP BY
555+
if query_in.group_by:
556+
query_traversal(query_in.group_by, add_column)
557+
558+
# Collect columns from HAVING
559+
if query_in.having:
560+
query_traversal(query_in.having, add_column)
561+
562+
# Collect columns from JOIN conditions
563+
for seq_item in join_sequence:
564+
if isinstance(seq_item, TableInfo) and seq_item.join_condition:
565+
query_traversal(seq_item.join_condition, add_column)
566+
567+
# If qualified star found for this table, fetch all columns
568+
if has_qualified_star_for_table:
569+
return None
570+
571+
# If we found no columns, fetch all
572+
if not columns:
573+
return None
574+
575+
# Convert column names to Identifier objects, we need to preserve quoting
576+
result = []
577+
for col, is_quoted in sorted(columns.items()):
578+
ident = Identifier(parts=[col])
579+
ident.is_quoted = [is_quoted]
580+
result.append(ident)
581+
return result
582+
434583
def process_table(self, item, query_in):
435584
table = copy.deepcopy(item.table)
436585
table.parts.insert(0, item.integration)
437586
table.is_quoted.insert(0, False)
438-
query2 = Select(from_table=table, targets=[Star()])
439-
# parts = tuple(map(str.lower, table_name.parts))
587+
588+
if self._can_prune_columns(item):
589+
needed_columns = self.get_columns_for_table(item, query_in, self.join_sequence)
590+
targets = needed_columns if needed_columns else [Star()]
591+
else:
592+
targets = [Star()]
593+
594+
query2 = Select(from_table=table, targets=targets)
440595
conditions = item.conditions
441596
if "or" in self.query_context["binary_ops"]:
442-
# not use conditions
443597
conditions = []
444598

445599
# For cross-database joins, skip the IN clause optimization

mindsdb/api/http/namespaces/databases.py

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import time
21
import shutil
32
import tempfile
43
from http import HTTPStatus
@@ -212,23 +211,12 @@ def put(self, database_name):
212211
new_integration = session.database_controller.get_integration(new_integration_id)
213212
return new_integration, HTTPStatus.CREATED
214213

215-
if check_connection:
216-
existing_integration = session.integration_controller.get(database_name)
217-
temp_name = f"{database_name}_{time.time()}".replace(".", "")
218-
try:
219-
handler = session.integration_controller.create_tmp_handler(
220-
temp_name, existing_integration["engine"], parameters
221-
)
222-
status = handler.check_connection()
223-
except ImportError as import_error:
224-
status = HandlerStatusResponse(success=False, error_message=str(import_error))
225-
226-
if status.success is not True:
227-
return http_error(
228-
HTTPStatus.BAD_REQUEST, "Connection error", status.error_message or "Connection error"
229-
)
214+
try:
215+
session.integration_controller.modify(database_name, parameters, check_connection=check_connection)
216+
except Exception as e:
217+
status = HandlerStatusResponse(success=False, error_message=str(e))
218+
return http_error(HTTPStatus.BAD_REQUEST, "Connection error", status.error_message or "Connection error")
230219

231-
session.integration_controller.modify(database_name, parameters)
232220
return session.integration_controller.get(database_name)
233221

234222
@ns_conf.doc("delete_database")

0 commit comments

Comments
 (0)