Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spatial queries by explicitly referencing table names #37

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

tkaratug
Copy link
Collaborator

@tkaratug tkaratug commented Feb 20, 2025

Overview

This PR builds upon and expands the changes introduced in #33. While the original PR addressed an issue with spatial queries, this PR refines the implementation further to ensure explicit table references in SELECT queries, reducing ambiguity and improving query reliability. As a result, PR #33 will be closed in favor of this one.

Changes

  • Updated scopeSelectDistanceTo and newQuery methods in HasSpatial to use {$this->getTable()}.* instead of *.
  • Updated test assertions in HasSpatialTest to reflect these changes.

Why?

  • Avoids ambiguity when joining multiple tables.
  • Ensures better SQL readability and maintainability.
  • Prevents unexpected query behavior due to implicit column selection.

Testing

  • Updated unit tests to assert queries include explicit table references.
  • All tests pass successfully.

Notes

This change is fully backward-compatible and does not alter existing behavior beyond ensuring explicit table references.

Checklist

  • Code changes reviewed and tested.
  • Unit tests updated accordingly.
  • No breaking changes introduced.

Previously, queries using `select *` could lead to ambiguous column issues
in complex queries. This update ensures that all queries explicitly reference
the table name (`{table}.*`) to prevent conflicts and improve SQL clarity.

Additionally, test assertions have been updated to reflect this change.
@tkaratug tkaratug requested a review from frkcn February 20, 2025 14:01
@tkaratug tkaratug self-assigned this Feb 20, 2025
@tkaratug tkaratug mentioned this pull request Feb 20, 2025
@tkaratug tkaratug merged commit 25c513e into main Feb 20, 2025
24 checks passed
tkaratug added a commit that referenced this pull request Feb 20, 2025
Fix spatial queries by explicitly referencing table names
tkaratug added a commit that referenced this pull request Feb 24, 2025
Merge pull request #37 from tarfin-labs/specifying-table-name
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