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

Caanot build arbitrary Queries without "from" for root Select #378

Closed
circulon opened this issue Feb 10, 2021 · 6 comments · Fixed by #381
Closed

Caanot build arbitrary Queries without "from" for root Select #378

circulon opened this issue Feb 10, 2021 · 6 comments · Fixed by #381
Labels
bug An existing feature is not working as intended

Comments

@circulon
Copy link
Contributor

Describe the bug
When using the query builder there must be a "table" or "from_" in the root of the query even if it is not used

To Reproduce

        builder = (
            QueryBuilder()
                .add_select(
                "other_test",
                lambda query: (
                    query.max("updated_at")
                        .from_("different_table")
                ),
            ).add_select(
                "some_alias",
                lambda query: (
                    query.max("updated_at")
                        .from_("another_table")
                ),
            )
        )

Should build a query along the lines of

SELECT (Select MAX(updated_at) as updated_at FROM different_table) as other_test, (Select MAX(updated_at) as updated_at FROM another_table) as some_alias;

Instead Throws an exception ending with

  File "/Users/test/code/venv/lib/python3.7/site-packages/masoniteorm/query/grammars/BaseGrammar.py", line 399, in process_table
    for t in table.split(".")
AttributeError: 'NoneType' object has no attribute 'split'

Note: Adding the following to the beginning of the chain compiles the query as expecetd

.select("*").from_("test")

Expected behavior
A valid query should be compiled

Desktop (please complete the following information):

  • OS: Mac OSX
  • Version Catalina 10.15.7

What database are you using?

  • Type: Postgres
  • Version 10.7

Additional context
masonite orm 1.0.22

@circulon circulon added the bug An existing feature is not working as intended label Feb 10, 2021
@josephmancuso
Copy link
Member

I don't think this is a bug? The root starting query builder needs a table, no?

@circulon
Copy link
Contributor Author

circulon commented Feb 10, 2021

Not necessarily a bug but definitely an issue

examples

Select NOW();
Output is the current date time
To get this same functionality with current QueryBuilder you have to do the following or equivalent
Select NOW() from some_valid_table LIMIT 1;
Note: in this case without the LIMIT you will end up with a collection of the the date for the number of rows in the table specified which is obviously undesirable.

select 1 where (Select count(*)>0 from some_table );
Output is 1

These are very basic examples but are valid nonetheless.

also by forcing the from when using multiple add_select for unrelated data points in a single row it moves some of the encapsulation that the subselects provide as an alias. ie its a little harder to read/understand IMHO

@josephmancuso
Copy link
Member

josephmancuso commented Feb 11, 2021

Writing the query out like this makes more sense:

SELECT
    (SELECT MAX(updated_at) AS updated_at FROM different_table) AS other_test,
    (SELECT MAX(updated_at) AS updated_at FROM another_table) AS some_alias;

@josephmancuso
Copy link
Member

Not all databases support this .. 🤔

@josephmancuso
Copy link
Member

@circulon Fixed this but i didn't add support to limits, wheres or group bys or anything .. I couldn't find any query examples where you would select without a table and have a limit, where or group bys. I can add this back if this is valid syntax but everything I read says selecting without a table isn't even really valid SQL lol. Maybe you can help me

@circulon
Copy link
Contributor Author

circulon commented Feb 12, 2021

Yeah that was probably a poor example to explain the point.

Apologies for not being more concise

I'm using Query builder very muck like the original example to add multiple subqueries with aliases.
These are metadata style queries for caching and so dont actually need a model as such.
In some cases I have 7-10 subqueries to get the metadata from 7+ tables in a single query.
On a purely coding level this fix allows me to add all queries by using a list of dicts which simplifies things greatly as only the alias and table names change.

I hope this clarifies things?

Happy to discuss things further if you like?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing feature is not working as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants