Skip to content

Allow trino catalog override#29

Closed
hladush wants to merge 1 commit intomainfrom
ihladush/allow_to_owerride_tirno_catalog
Closed

Allow trino catalog override#29
hladush wants to merge 1 commit intomainfrom
ihladush/allow_to_owerride_tirno_catalog

Conversation

@hladush
Copy link
Contributor

@hladush hladush commented Jul 24, 2025

Trino allows connections to different RDS instances by configuring a separate catalog for each database. Each catalog can only connect to a single database. In current Heimdall implementation each cluster holds different catalog and as a result if a new Trino catalog was added new cluster and command should be added to config.

To improve the user experience, we propose allowing users to dynamically override the target catalog as part of the task definition, rather than statically configuring all catalogs in advance.

This enhancement was tested successfully on a local build.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables dynamic catalog override functionality for Trino connections, allowing users to specify a different catalog in their job definition rather than being limited to the statically configured cluster catalog.

  • Add optional Catalog field to job context for dynamic catalog specification
  • Implement catalog override logic that prioritizes job-level catalog over cluster-level catalog
  • Update Trino request headers to use the resolved catalog value

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/pkg/object/command/trino/trino.go Adds optional Catalog field to jobContext struct
internal/pkg/object/command/trino/client.go Implements catalog override logic and updates request header usage

return nil, err
}
}

Copy link

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The code assumes clusterCtx.Catalog exists but there's no validation that clusterCtx is not nil or that the Catalog field is populated. This could result in an empty catalog being used if the cluster configuration is missing or incomplete.

Suggested change
// Validate clusterCtx and its Catalog field
if clusterCtx == nil || clusterCtx.Catalog == "" {
return nil, fmt.Errorf("invalid cluster context: missing or empty Catalog")
}

Copilot uses AI. Check for mistakes.
@hladush
Copy link
Contributor Author

hladush commented Jul 24, 2025

It was decided to specify catalog name inside select * from catalog_name.schema.table_name

@hladush hladush closed this Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants