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

Generated SQL does not correspond to intended SQL in examples\exists.cpp #922

Closed
juandent opened this issue Feb 26, 2022 · 11 comments
Closed
Labels

Comments

@juandent
Copy link
Contributor

juandent commented Feb 26, 2022

Intended SQL is as documented in exists.cpp like so:

SELECT agent_code,agent_name,working_area,commission
FROM agents
WHERE exists
(SELECT *
FROM customer
WHERE grade=3 AND agents.agent_code=customer.agent_code)
ORDER BY commission;

To produce this EXACT SQL we need to modify original sqlite_orm code to the following:

        auto statement = storage.prepare(select(columns(&Agent::code, &Agent::name, &Agent::workingArea, &Agent::comission), from<Agent>(),
                                   where(exists(select(asterisk<Customer>(),from<Customer>(),
                                                       where(is_equal(&Customer::grade, 3) and
                                                             is_equal(&Agent::code, &Customer::agentCode))))),
                                   order_by(&Agent::comission)));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

where sql contains

SELECT "agents"."AGENT_CODE", "agents"."AGENT_NAME", "agents"."WORKING_AREA", "agents"."COMMISSION" 
FROM 'agents' 
WHERE (EXISTS (
    SELECT * 
    FROM 'customer' 
    WHERE ((("customer"."GRADE" = NULL) AND ("agents"."AGENT_CODE" = "customer"."AGENT_CODE"))))) 
    ORDER BY "agents"."COMMISSION"  

and the resultset in variable rows contains 4 rows containing the agent codes A002, A009, A008, A010.

If we remove the explicit from from the inner subquery like this:

        auto statement = storage.prepare(select(columns(&Agent::code, &Agent::name, &Agent::workingArea, &Agent::comission), from<Agent>(),
                                   where(exists(select(asterisk<Customer>(), 
                                                       where(is_equal(&Customer::grade, 3) and
                                                             is_equal(&Agent::code, &Customer::agentCode))))),
                                   order_by(&Agent::comission)));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

the generated SQL becomes:

SELECT "agents"."AGENT_CODE", "agents"."AGENT_NAME", "agents"."WORKING_AREA", "agents"."COMMISSION" 
FROM 'agents' 
WHERE (EXISTS (
    SELECT * 
    FROM 'agents', 'customer' 
    WHERE ((("customer"."GRADE" = 3) AND ("agents"."AGENT_CODE" = "customer"."AGENT_CODE"))))) 
    ORDER BY "agents"."COMMISSION"  

which differs from the original intended query (the first SQL query above).
rows contains 12 rows, that is it does not filter any agent (Database contains 12 agents)...

If we now remove the explicit from from the outer query se obtain the following SQL string:

SELECT "agents"."AGENT_CODE", "agents"."AGENT_NAME", "agents"."WORKING_AREA", "agents"."COMMISSION" FROM 'agents', 'customer' WHERE (EXISTS (SELECT * FROM 'agents', 'customer' WHERE ((("customer"."GRADE" = 3) AND ("agents"."AGENT_CODE" = "customer"."AGENT_CODE"))))) ORDER BY "agents"."COMMISSION"  

with this C++ code:

        auto statement = storage.prepare(select(columns(&Agent::code, &Agent::name, &Agent::workingArea, &Agent::comission), 
                                   where(exists(select(asterisk<Customer>(),       
                                                       where(is_equal(&Customer::grade, 3) and
                                                             is_equal(&Agent::code, &Customer::agentCode))))),
                                   order_by(&Agent::comission)));

and the rows variable contains a resultset with 300 rows which is 25 customers X 12 agents, that is a CROSS JOIN that does not filter anything!!

IN CONCLUSION: the smart generation of the from clause does not work correctly either in outer or inner queries and dangerously sends a message of correctness that is not REAL. As per this moment, or the Feb 10 2022 version of the dev branch of sqlite_orm, it is necessary to include EXPLICIT from clauses for all queries that have subqueries! In particular, all examples provided share this common bug and puts in danger the semantics of the queries therein programmed.

Please take a serious look at this Eugene because it is not working as one would expect.

Your friend,
Juan Dent

@fnc12
Copy link
Owner

fnc12 commented Feb 27, 2022

Ok I see. What do you offer?

@fnc12 fnc12 added the question label Feb 27, 2022
@juandent
Copy link
Contributor Author

juandent commented Feb 27, 2022

The ideal solution would be to check the algorithm that inserts the extra table references and correct it... but I don't know how hard that would be; so at least all the examples should be updated to use explicit from clauses so the generated SQL agrees with the intended SQL

@juandent
Copy link
Contributor Author

currently, anyone new to sqlite_orm will try the examples and will find them in error and may think the whole library is unreliable... this is dangerous to the continued success of sqlite_orm

@fnc12
Copy link
Owner

fnc12 commented Feb 27, 2022

ok. Let's fix examples. Current solution works for both cases. Sometimes people need to mention two tables inside FROM, sometimes - only one. That is why I don't see a way of changing algorithm

@juandent
Copy link
Contributor Author

juandent commented Feb 27, 2022 via email

@fnc12
Copy link
Owner

fnc12 commented Feb 27, 2022

Yes. If you know what can be improved in examples please share it with me - I'll fix it. Thanks

@juandent
Copy link
Contributor Author

Hi!
in exists.cpp:
add from in outer select:

        //  SELECT agent_code,ord_num,ord_amount,cust_code
        //  FROM orders
        //  WHERE NOT EXISTS
        //      (SELECT agent_code
        //      FROM customer
        //      WHERE payment_amt=1400);
        auto statement = storage.prepare(select(
            columns(&Order::agentCode, &Order::num, &Order::amount, &Order::custCode), from<Order>(), 
            where(not exists(select(&Customer::agentCode, where(is_equal(&Customer::paymentAmt, 1400)))))));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

and also add this sample to exists.cpp:

        /*
        SELECT "orders"."AGENT_CODE", "orders"."ORD_NUM", "orders"."ORD_AMOUNT", "orders"."CUST_CODE", 'c'."PAYMENT_AMT"
            FROM 'orders' INNER JOIN  'customer' 'c' ON('c'."AGENT_CODE" = "orders"."AGENT_CODE")
            WHERE(NOT(EXISTS
                (
                    SELECT 'd'."AGENT_CODE" FROM 'customer' 'd' WHERE((('c'."PAYMENT_AMT" = 7000) AND('d'."AGENT_CODE" = 'c'."AGENT_CODE")))))
                )
            ORDER BY 'c'."PAYMENT_AMT"
        */
        using als = alias_c<Customer>;
        using als_2 = alias_d<Customer>;

        double amount = 2000;
        auto where_clause = select(alias_column<als_2>(&Customer::agentCode), from<als_2>(),
                where(is_equal(alias_column<als>(&Customer::paymentAmt), std::ref(amount)) and (alias_column<als_2>(&Customer::agentCode) == c(alias_column<als>(&Customer::agentCode)))));

        amount = 7000;

        auto statement = storage.prepare(select(
            columns(&Order::agentCode, &Order::num, &Order::amount, &Order::custCode, alias_column<als>(&Customer::paymentAmt)), from<Order>(),
            inner_join<als>(on(alias_column<als>(&Customer::agentCode) == c(&Order::agentCode))), where(not exists(where_clause)), order_by(alias_column<als>(&Customer::paymentAmt))));
        
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

Regards,
Juan

@juandent
Copy link
Contributor Author

A query in examples\subquery.cpp needs to include from<> in the outer query:

        //  SELECT first_name, last_name, department_id
        //  FROM employees
        //  WHERE department_id IN
        //      (SELECT DEPARTMENT_ID FROM departments
        //      WHERE location_id=1700);
        auto statement = storage.prepare(select(
            columns(&Employee::firstName, &Employee::lastName, &Employee::departmentId), from<Employee>(),
            where(in(&Employee::departmentId, select(&Department::id, where(c(&Department::locationId) == 1700))))));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

also this one:

        //  SELECT first_name, last_name, department_id
        //  FROM employees
        //  WHERE department_id NOT IN
        //  (SELECT DEPARTMENT_ID FROM departments
        //      WHERE manager_id
        //      BETWEEN 100 AND 200);
        auto statement =
            storage.prepare(select(columns(&Employee::firstName, &Employee::lastName, &Employee::departmentId),from<Employee>(),
                           where(not_in(&Employee::departmentId,
                                        select(&Department::id, where(between(&Department::managerId, 100, 200)))))));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

and this one too:

        //  SELECT last_name, salary, department_id
        //  FROM employees e
        //  WHERE salary >(SELECT AVG(salary)
        //      FROM employees
        //      WHERE department_id = e.department_id);
        using als = alias_e<Employee>;
        auto statement = storage.prepare(select(
            columns(alias_column<als>(&Employee::lastName),
                    alias_column<als>(&Employee::salary),
                    alias_column<als>(&Employee::departmentId)),
            from<als>(),
            where(greater_than(
                alias_column<als>(&Employee::salary),
                select(avg(&Employee::salary),
                       where(is_equal(&Employee::departmentId, alias_column<als>(&Employee::departmentId))))))));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

and this one too:

        //  SELECT first_name, last_name, employee_id, job_id
        //  FROM employees
        //  WHERE 1 <=
        //      (SELECT COUNT(*) FROM Job_history
        //      WHERE employee_id = employees.employee_id);
        auto statement =
            storage.prepare(select(columns(&Employee::firstName, &Employee::lastName, &Employee::id, &Employee::jobId), from<Employee>(),
                           where(lesser_or_equal(
                               1,
                               select(count<JobHistory>(), from<JobHistory>(), where(is_equal(&Employee::id, &JobHistory::employeeId)))))));
        auto sql = statement.expanded_sql();
        auto rows = storage.execute(statement);

That's it for subquery.cpp.

Regards,
Juan

@juandent
Copy link
Contributor Author

Suggestion: by preparing the statement one has access to "expanded_sql()" which lets one compare the generated SQL with the intended... I propose that all non-trivial selects are done this way so it is easy to verify reality against intended SQL
Its good for debugging and also for development: making sure all our SQL is what we want!

@fnc12
Copy link
Owner

fnc12 commented Mar 1, 2022

#927 thanks Juan!

@fnc12
Copy link
Owner

fnc12 commented Mar 2, 2022

merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants