The group by clause is not working when used with sp and passed as a parameter

Question

I have created a stored procedure for the purpose of optimization. Below is the actual code.

DELIMITER $$
CREATE PROCEDURE getImpressions (
    IN aff_id BIGINT(20),
    IN funn_id BIGINT(20),
    IN grpBy TEXT,
    IN odrBy TEXT
)
BEGIN
    SELECT
        affiliate_id,
        funnel_id,
        COUNT(DISTINCT (decimal_ip)) as no_of_records,
        HOUR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern')) AS hour,
        CEIL(HOUR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern'))/2) AS hoursby2,
        DATE(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern')) AS date,
        WEEK(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern')) AS week,
        CEIL(WEEK(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern'))/2) AS weeksby2,
        MONTH(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern')) AS month,
        YEAR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), 'UTC', 'US/Eastern')) AS year
    FROM gs_aff_analytics
    WHERE affiliate_id = aff_id AND funnel_id = funn_id
    GROUP BY grpBy
    ORDER BY odrBy
;

END$$

DELIMITER ;

#calling the sp
CALL getImpressions(36, 2, 'date', 'date');

Here everything is working fine. But the grpBy value which I'm passing through parameter is not working. This means the sp just not taking it even if I'm passing it. But as soon as I recreate the sp and explicitly write the group by clause as GROUP BY date (hard coding the group by) it start working properly.

look at this question: https://stackoverflow.com/questions/8857090/group-by-using-parameters-in-sql — chris-crush-code, Sep 18 '20 at 14:53

spencer7593 · Answer 1 · 2020-09-18T15:13:54.457

1

In the context of the SQL statement, grpBy is a scalar value. The behavior is similar to including a literal value in place of grpBu

If we modify the SQL statement to replace grpBy with a literal value, e.g.

GROUP BY 'some_literal_string_value'

we would get an equivalent result.

grpBy is not seen as a column name. It's not seen as a SQL expression that references any identifier. (This isn't specific to the GROUP BY clause, this rule applies everywhere in the SQL statement.)

To get expressions/identifiers dynamically included in the SQL text, we would need to use create a string containing the SQL text, and then execute with dynamic SQL. Note that this approach can open up a huge SQL injection vulnerability...

SET @sql = CONCAT('SELECT ... GROUP BY ',grpBy,' ... ');  
PREPARE stmt FROM @sql;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;

https://dev.mysql.com/doc/refman/8.0/en/sql-prepared-statements.html

edited Sep 18 '20 at 15:13

answered Sep 18 '20 at 14:55

spencer7593

106,611
15
112
140

You are saying, I need to use CONCAT and get lined up the SQL and then execute it. Then how the other parameters like `affilaite_id = aff_id` are working absolutely fine? – Ankit Singh Sep 18 '20 at 15:06
the others are working because those are scalar values. those are not seen as column names. it's seen the same as `affiilate_id = 'some_value'` it's not seen as `affiliate_id = column_name`. the GROUP BY clause is working the same way `GROUP BY some_value`. In either case, if we want to inject an *identifier* (e.g. a column name) into the SQL text, we need to create a SQL string, and then execute that dynamically. – spencer7593 Sep 18 '20 at 15:17
this restriction has to do with how SQL gets executed, the set operations that are performed for a SQL statement... parse tokens, syntax check, semantic check, derive execution plan, and execute, ... those operations require that the *identifiers* (table names, column_names) must be part of the SQL text, they can not be provided as *values*. (For example, the semantic check needs to know what tables and columns are referenced, are those valid, does the user have required privileges, etc., that check has to be done at prepare time, and can't be deferred to execution time like values. – spencer7593 Sep 18 '20 at 15:21
I'm not saying you need to use the `CONCAT` function. I was giving an example of dynamic SQL, one approach to getting column names dynamically incorporated into the text of a SQL statement. In terms of MySQL, there's nothing wrong with doing a `GROUP BY 'literal'`, it's valid to do that. The issue appears to be that you want `'literal'` to be seen as a column name. – spencer7593 Sep 18 '20 at 15:26
Yes, exactly. So is there a way to parse that `value` as a `literal`. As I'm not getting any way to do so inside the sp. – Ankit Singh Sep 18 '20 at 15:34
That `CONCAT` worked actually. So is there a way to prevent SQL Injection. Although, I can see some `PREPARE stmt` in your answer. – Ankit Singh Sep 18 '20 at 18:18
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/221703/discussion-between-ankit-singh-and-spencer7593). – Ankit Singh Sep 18 '20 at 18:25

Ankit Singh · Answer 2 · 2020-09-18T19:02:24.023

Here is the working solution

DELIMITER $$
CREATE PROCEDURE getImpressions (
    IN typ TEXT,
    IN aff_id BIGINT(20),
    IN funn_id BIGINT(20),
    IN fromTimestamp BIGINT(20),
    IN toTimestamp BIGINT(20),
    IN grpBy TEXT,
    IN odrBy TEXT
)
BEGIN
    DECLARE strQuery text;
    DECLARE stmtp text;

    IF(typ = 'all')
        THEN SET  @strQuery = 'COUNT(1) AS no_of_records,';
    END IF;

    IF(typ = 'unique')
        THEN SET  @strQuery = 'COUNT(DISTINCT (decimal_ip)) AS no_of_records,';
    END IF;

    SET @strQuery = CONCAT('
        SELECT ', 
            @strQuery,
            '
            HOUR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\')) AS hour,
            CEIL(HOUR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\'))/2) AS hoursby2,
            DATE(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\')) AS date,
            WEEK(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\')) AS week,
            CEIL(WEEK(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\'))/2) AS weeksby2,
            MONTH(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\')) AS month,
            YEAR(CONVERT_TZ(FROM_UNIXTIME(created_timestamp), \'UTC\', \'US/Eastern\')) AS year,
            created_timestamp
        FROM gs_aff_analytics
        WHERE affiliate_id = ', aff_id, ' AND funnel_id = ', funn_id);
    
    IF(fromTimestamp IS NOT NULL)
        THEN SET @strQuery = CONCAT(@strQuery, ' AND created_timestamp BETWEEN ', fromTimestamp);
    END IF;

    IF(toTimestamp IS NOT NULL)
        THEN SET @strQuery = CONCAT(@strQuery, ' AND created_timestamp <= ', toTimestamp);
    END IF;

    IF (grpBy IS NOT NULL)
        THEN SET @strQuery = CONCAT(@strQuery, ' GROUP BY ', grpBy);
    END IF;
 
    IF (odrBy IS NOT NULL)
        THEN SET @strQuery = CONCAT(@strQuery, ' ORDER BY ', odrBy);
    END IF;

    PREPARE  stmtp FROM  @strQuery;
    EXECUTE  stmtp;

END$$

DELIMITER ;

#calling the sp
CALL getAllImpressions('all', 36, 2, null, null, 'week', 'week');
CALL getAllImpressions('unique', 36, 2, 0, 1600000000, 'week', 'week');

The group by clause is not working when used with sp and passed as a parameter

2 Answers2