Skip to content

Conversation

@KariHall619
Copy link
Contributor

Which issue(s) this PR fixes:
Fixes #700

What type of PR is this?
Fix: Improve error reporting for unimplemented Query in loaders

What this PR does / why we need it:
This PR refactors the Query methods in pkg/testing/loader_file.go and pkg/testing/loader_non.go.
Previously, these methods did not clearly indicate their inability to perform database queries in a way that aligns with Go's error handling conventions:
fileLoader.Query returned a nil error while passing a "not support" message through the data payload.
nonLoader.Query returned a nil error and an empty result, failing silently.
These behaviors made it harder for calling code to reliably determine if a query was unsupported and could lead to confusion during debugging.
This change modifies both methods to return an explicit, non-nil error (e.g., errors.New("Query functionality is not yet implemented for fileLoader")). This provides:

  1. Clearer Error Indication: Callers can use the standard if err != nil check.
  2. Adherence to Go Conventions: Aligns with idiomatic Go error handling.
  3. Improved Debuggability: Explicit errors make it easier to understand limitations.
  4. This PR does not implement query functionality for these loaders but improves the clarity of their current unimplemented state.

The Query methods in fileLoader and nonLoader previously either
returned a "not support" message via the data payload (fileLoader)
or failed silently (nonLoader), both with a nil error.

This commit changes them to return explicit non-nil errors,
indicating that the Query functionality is not implemented for
these loader types. This aligns better with Go's error handling
conventions and provides clearer feedback to callers.
@sonarqubecloud
Copy link

Copy link
Owner

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

Hi @KariHall619 , thanks for your effort. But please upload some screenshots about the test process. I guess you already tested it on the web UI.

Copy link
Owner

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

image

It works well.

@LinuxSuRen LinuxSuRen merged commit b572c78 into LinuxSuRen:master May 29, 2025
14 checks passed
@KariHall619 KariHall619 deleted the fix-700 branch September 10, 2025 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Database Query Fails for Stores with Special Characters in Name or Properties

2 participants