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

Use table alias/name when building date query #9700

Closed
wants to merge 1 commit into from

Conversation

jkudish
Copy link

@jkudish jkudish commented Nov 28, 2023

This PR attempts to fix the issue described in #9699

Use the received table name & table alias arguments in the get_sql function to set those as properties on the Date class. Note that get_sql already gets called with those params as per the line below. However the get_sql function oddly wasn't defining those arguments in its definition.

$clauses = $this->date_query->get_sql( $this->table_name, $this->table_alias, $primary, $this );

Then, when building the SQL query, use the alias or name (only if available) to prepend to the column name in the query. This prevents ambiguity issues when performing the SQL query

This prevents and fixes the issue described in #9699

I am open to receiving feedback on the solution proposed and working on an updated solution.

Thanks!

use the received table name & table alias arguments in the `get_sql` function to set those as properties on the Date class.

Then, when building the SQL query, use the alias or name (only if available) to prepend to the column name in the query.

This prevents and fixes the issue described in awesomemotive#9699
@robincornett
Copy link
Contributor

Hi @jkudish, and thank you so much for looking into this and submitting a PR! I'll check it over locally, but I wanted to ask if you would be willing to also submit the issue and PR over in the BerlinDB repository? Berlin is the package/library we are using to manage our custom database tables, and based on what you're saying, it looks like this is a fix that should be applied in there.

@jkudish
Copy link
Author

jkudish commented Dec 5, 2023

Apologies for the delay. I've moved this over to berlindb/core#164 as suggested

@jkudish jkudish closed this Dec 5, 2023
@jkudish jkudish deleted the fix/issue-9699 branch December 5, 2023 06:32
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