parser: semantic: validate table aliases for SELECT statements

PauloMigAlmeida · Oct 10, 2023 · f27e9cb · f27e9cb
1 parent f55c81c
commit f27e9cb
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 18 deletions.
diff --git a/src/parser/midorisql.y b/src/parser/midorisql.y
@@ -323,14 +323,14 @@ insert_expr:
    | NULLX         { emit(result, "NULL"); }
    ;
 
-insert_expr: insert_expr '+' insert_expr { emit(result, "ADD"); }
-   | insert_expr '-' insert_expr         { emit(result, "SUB"); }
-   | insert_expr '*' insert_expr         { emit(result, "MUL"); }
-   | insert_expr '/' insert_expr         { emit(result, "DIV"); }
-   | insert_expr '%' insert_expr         { emit(result, "MOD"); }
-   | '-' insert_expr %prec UMINUS        { emit(result, "NEG"); }      
-   | '(' insert_expr ')'
-   ;  
+insert_expr: insert_expr '+' insert_expr	{ emit(result, "ADD"); }
+	   | insert_expr '-' insert_expr        { emit(result, "SUB"); }
+	   | insert_expr '*' insert_expr        { emit(result, "MUL"); }
+	   | insert_expr '/' insert_expr        { emit(result, "DIV"); }
+	   | insert_expr '%' insert_expr        { emit(result, "MOD"); }
+	   | '-' insert_expr %prec UMINUS       { emit(result, "NEG"); }      
+	   | '(' insert_expr ')'
+	   ;
 
    /** update table **/
 stmt: update_stmt { emit(result, "STMT"); }

diff --git a/src/parser/semantic_select.c b/src/parser/semantic_select.c
@@ -27,20 +27,35 @@ static bool do_build_alias_map(struct ast_node *root, char *out_err, size_t out_
 {
 	struct list_head *pos;
 	struct ast_node *tmp_entry;
+	struct ast_sel_alias_node *alias_node;
 	struct ast_sel_table_node *table_node;
 
 	if (root->node_type == AST_TYPE_SEL_ALIAS) {
-		list_for_each(pos, root->node_children_head)
+		alias_node = (typeof(alias_node))root;
+
+		list_for_each(pos, alias_node->node_children_head)
 		{
 			tmp_entry = list_entry(pos, typeof(*tmp_entry), head);
 
 			if (tmp_entry->node_type == AST_TYPE_SEL_TABLE) {
 				table_node = (typeof(table_node))tmp_entry;
-				char *key = table_node->table_name;
+				char *key = alias_node->alias_value;
+				char *value = table_node->table_name;
 
-				// TODO check for duplicates
+				/* is the name valid? table aliases follow table nomenclature rules */
+				if (!table_validate_name(key)) {
+					snprintf(out_err, out_err_len, "alias name '%.128s' is invalid\n", key);
+					return false;
+				}
+
+				/* was this alias already declared ? */
+				if (hashtable_get(out_ht, key, strlen(key) + 1)) {
+					// fun fact, SQLITE doesn't validate if aliases are unique
+					snprintf(out_err, out_err_len, "Not unique table/alias: '%.128s'\n", key);
+					return false;
+				}
 
-				if (!hashtable_put(out_ht, key, strlen(key) + 1, key, strlen(key) + 1)) {
+				if (!hashtable_put(out_ht, key, strlen(key) + 1, value, strlen(value) + 1)) {
 					snprintf(out_err, out_err_len, "semantic phase: internal error\n");
 					return false;
 				}
@@ -66,14 +81,10 @@ static bool build_table_alias_map(struct ast_node *root, char *out_err, size_t o
 		goto err;
 
 	if (!do_build_alias_map(root, out_err, out_err_len, out_ht))
-		goto err_ht_put_col;
+		return false;
 
 	return true;
 
-err_ht_put_col:
-	/* clean up */
-	hashtable_foreach(out_ht, &free_hashmap_entries, NULL);
-	hashtable_free(out_ht);
 err:
 	snprintf(out_err, out_err_len, "semantic phase: internal error\n");
 	return false;
@@ -117,6 +128,7 @@ bool semantic_analyse_select_stmt(struct database *db, struct ast_node *node, ch
 	bool ret = true;
 	struct hashtable ht = {0};
 
+	/* build table alias hashtable; check for duplicates */
 	if (!(ret = build_table_alias_map(node, out_err, out_err_len, &ht)))
 		goto cleanup;
 
@@ -126,7 +138,7 @@ bool semantic_analyse_select_stmt(struct database *db, struct ast_node *node, ch
 
 	/*
 	 * TODO:
-	 * - Check for duplicate / invalid aliases
+	 * - Check for duplicate / invalid aliases (table done, columns tbi)
 	 * - check field name (full qualified ones) as they can contain either table name or alias
 	 * - check for columns existence
 	 * - check for ambiguous columns on join statements

diff --git a/tests/parser/semantic.c b/tests/parser/semantic.c
@@ -639,6 +639,14 @@ static void select_tests(void)
 	helper(&db, "SELECT f1, f2, f3 FROM V_A_1, V_A_2, V_A_3;", false);
 	helper(&db, "SELECT * FROM V_A_1 JOIN V_A_2 ON f1 = f2 JOIN V_A_3 ON f2 = f3;", false);
 
+	/* valid case - table aliases */
+	prep_helper(&db, "CREATE TABLE V_B_1 (f1 INT);");
+	prep_helper(&db, "CREATE TABLE V_B_2 (f2 INT);");
+	prep_helper(&db, "CREATE TABLE V_B_3 (f2 INT);");
+	helper(&db, "SELECT f1 FROM V_B_1 as v;", false);
+	helper(&db, "SELECT v1.f1, v2.f2, f3 FROM V_B_1 as v1, V_B_2 as v2, V_B_3;", false);
+	helper(&db, "SELECT * FROM V_B_1 v1 JOIN V_B_2 v2 ON v2.f1 = v2.f2 JOIN V_B_3 ON v2.f2 = f3;", false);
+
 	/* invalid case - table doesn't exist */
 	prep_helper(&db, "CREATE TABLE I_A_1 (f1 INT);");
 	prep_helper(&db, "CREATE TABLE I_A_2 (f2 INT);");
@@ -647,6 +655,13 @@ static void select_tests(void)
 	helper(&db, "SELECT f1, f2, f3 FROM I_A_1, I_A_2, I_A_31;", true);
 	helper(&db, "SELECT * FROM I_A_1 JOIN I_A_2 ON f1 = f2 JOIN I_A_31 ON f2 = f3;", true);
 
+	/* invalid case - table aliases duplicated */
+	prep_helper(&db, "CREATE TABLE I_B_1 (f1 INT);");
+	prep_helper(&db, "CREATE TABLE I_B_2 (f2 INT);");
+	prep_helper(&db, "CREATE TABLE I_B_3 (f2 INT);");
+	helper(&db, "SELECT v1.f1, v2.f2, f3 FROM I_B_1 as v1, I_B_2 as v1, I_B_3;", true);
+	helper(&db, "SELECT * FROM I_B_1 v1 JOIN I_B_2 v1 ON v2.f1 = v2.f2 JOIN I_B_3 ON v2.f2 = f3;", true);
+
 	database_close(&db);
 }