Skip to content

Conversation

sanketjadhavSF
Copy link
Contributor

@sanketjadhavSF sanketjadhavSF commented Jul 25, 2025

This pull request introduces a new PostgreSQL plugin for executing SQL queries and managing database interactions. The changes include the implementation of the core plugin functionality, a detailed README for usage and configuration, and an entry point for integrating the plugin.

PostgreSQL Plugin Implementation:

  • Added internal/pkg/object/command/postgres/postgres.go to implement the PostgreSQL plugin, supporting synchronous and asynchronous query execution, error handling, and transaction management. It includes helper functions like splitAndTrimQueries for query parsing and methods to handle query execution (handleSyncQuery and handleAsyncQueries).

Documentation:

  • Created plugins/postgres/README.md to provide comprehensive documentation for the PostgreSQL plugin. It includes features, configuration details, execution modes, and usage examples for both single and batch queries.

Integration:

  • Added plugins/postgres/postgres.go as the entry point to initialize the PostgreSQL plugin by invoking the New function from the core implementation.

Local Test Results:

  • Sync Query Execution:

Submit Query:
image
Get Result:
image

  • Async Query Execution (for batch processing multiple sql statements):
    Submit Queries:
image

Get Result (No results returned in case of async just a completion message):
image


func handleSyncQuery(db *database.Database, query string, j *job.Job) error {
// Allow a single query, even if it ends with a semicolon
queries := splitAndTrimQueries(query)
Copy link
Contributor

Choose a reason for hiding this comment

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

does postres not allow to call query with semicolon at the end?

Copy link
Contributor Author

@sanketjadhavSF sanketjadhavSF Jul 28, 2025

Choose a reason for hiding this comment

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

this splitAndTrimQueries() function is used to parse the query param and find if there are multiple queries based on ; separator and route the query execution to different function executeSyncQuery() or executeAsyncQueries()
executeSyncQuery - this function is executed when return_result is set to true and only single query is executed.
executeAsyncQueries - this is used to run .sql scripts with multiple sql statements. this will return only errors if any and not results are returned.

// Handler for the PostgreSQL query execution.
func (p *postgresCommandContext) handler(r *plugin.Runtime, j *job.Job, c *cluster.Cluster) error {
jobContext := &postgresJobContext{}
if j.Context != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's move it to 2 functions

  • create and validate job context
  • Create an validate cluster context

In that case method vill be a little bit clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done added separate functions for job and cluster context validation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also added the test results from local to PR description

hladush
hladush previously approved these changes Jul 28, 2025
Copy link
Contributor

@hladush hladush left a comment

Choose a reason for hiding this comment

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

Please add testing section in PR

@sanketjadhavSF sanketjadhavSF marked this pull request as ready for review July 28, 2025 16:21
@sanketjadhavSF sanketjadhavSF changed the title Add PostgreSQL plugin Add PostgreSQL plugin configs for Cluster and Command Jul 28, 2025
@sanketjadhavSF sanketjadhavSF changed the title Add PostgreSQL plugin configs for Cluster and Command Add PostgreSQL plugin Jul 28, 2025
@Copilot Copilot AI review requested due to automatic review settings August 5, 2025 05:25
Copy link
Contributor

@Copilot 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 pull request introduces a new PostgreSQL plugin for Heimdall that enables SQL query execution and database management. The plugin supports both synchronous query execution (with result return) and asynchronous batch processing modes.

  • Implements core PostgreSQL plugin functionality with sync/async query execution modes
  • Provides comprehensive documentation with configuration examples and usage guidelines
  • Creates plugin entry point for integration with the Heimdall system

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
plugins/postgres/postgres.go Entry point that initializes the PostgreSQL plugin by calling the core implementation
plugins/postgres/README.md Comprehensive documentation covering features, configuration, and usage examples
internal/pkg/object/command/postgres/postgres.go Core plugin implementation with query execution, validation, and error handling

return err
}

db := &database.Database{ConnectionString: clusterContext.ConnectionString}
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The connection string is being passed directly without validation or sanitization. Consider validating the connection string format and ensuring it doesn't contain malicious parameters that could lead to security vulnerabilities.

Copilot uses AI. Check for mistakes.

}
defer sess.Close()

_, err = sess.Exec(query)
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

Executing raw SQL queries without prepared statements or input validation could expose the system to SQL injection attacks. Consider using parameterized queries or implementing input validation for the query parameter.

Copilot uses AI. Check for mistakes.

}
defer sess.Close()

rows, err := sess.Query(queries[0])
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

Executing raw SQL queries without prepared statements or input validation could expose the system to SQL injection attacks. Consider using parameterized queries or implementing input validation for the query parameter.

Copilot uses AI. Check for mistakes.


func splitAndTrimQueries(query string) []string {
queries := []string{}
for _, q := range strings.Split(query, ";") {
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The query splitting logic using semicolon delimiter is naive and could incorrectly split queries that contain semicolons within string literals, comments, or function definitions. This could break valid SQL statements or create security vulnerabilities.

Copilot uses AI. Check for mistakes.

Comment on lines +111 to +133
func (p *postgresCommandContext) executeAsyncQueries(db *database.Database, query string, j *job.Job) error {
sess, err := db.NewSession(false)
if err != nil {
j.Result = &result.Result{
Columns: []*column.Column{{
Name: "error",
Type: column.Type("string"),
}},
Data: [][]any{{fmt.Sprintf("Async PostgreSQL connection error: %v", err)}},
}
return fmt.Errorf("Async PostgreSQL connection error: %v", err)
}
defer sess.Close()

_, err = sess.Exec(query)
if err != nil {
j.Result = &result.Result{
Columns: []*column.Column{{
Name: "error",
Type: column.Type("string"),
}},
Data: [][]any{{fmt.Sprintf("Async PostgreSQL query execution error: %v", err)}},
}
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The error result construction is duplicated in both error handling blocks (lines 114-120 and 127-133). Consider extracting this into a helper function to reduce code duplication and improve maintainability.

Suggested change
func (p *postgresCommandContext) executeAsyncQueries(db *database.Database, query string, j *job.Job) error {
sess, err := db.NewSession(false)
if err != nil {
j.Result = &result.Result{
Columns: []*column.Column{{
Name: "error",
Type: column.Type("string"),
}},
Data: [][]any{{fmt.Sprintf("Async PostgreSQL connection error: %v", err)}},
}
return fmt.Errorf("Async PostgreSQL connection error: %v", err)
}
defer sess.Close()
_, err = sess.Exec(query)
if err != nil {
j.Result = &result.Result{
Columns: []*column.Column{{
Name: "error",
Type: column.Type("string"),
}},
Data: [][]any{{fmt.Sprintf("Async PostgreSQL query execution error: %v", err)}},
}
// errorResult constructs a result.Result containing a single error message.
func errorResult(msg string) *result.Result {
return &result.Result{
Columns: []*column.Column{{
Name: "error",
Type: column.Type("string"),
}},
Data: [][]any{{msg}},
}
}
func (p *postgresCommandContext) executeAsyncQueries(db *database.Database, query string, j *job.Job) error {
sess, err := db.NewSession(false)
if err != nil {
j.Result = errorResult(fmt.Sprintf("Async PostgreSQL connection error: %v", err))
return fmt.Errorf("Async PostgreSQL connection error: %v", err)
}
defer sess.Close()
_, err = sess.Exec(query)
if err != nil {
j.Result = errorResult(fmt.Sprintf("Async PostgreSQL query execution error: %v", err))

Copilot uses AI. Check for mistakes.

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