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

Can not use stored procedure with arguments #1496

Closed
cipharius opened this issue Mar 30, 2021 · 3 comments · Fixed by dolthub/go-mysql-server#354
Closed

Can not use stored procedure with arguments #1496

cipharius opened this issue Mar 30, 2021 · 3 comments · Fixed by dolthub/go-mysql-server#354
Assignees

Comments

@cipharius
Copy link

Given the example setup:

CREATE TABLE items (
  id INT PRIMARY KEY AUTO_INCREMENT,
  item TEXT NOT NULL
);

CREATE PROCEDURE add_item (IN txt TEXT) MODIFIES SQL DATA
INSERT INTO items (item) VALUES (txt);

CALL add_item('A test item');

Dolt throws an error:

error on line 6 for query 

CREATE PROCEDURE add_item (IN txt TEXT) MODIFIES SQL DATA
INSERT INTO items (item) VALUES (txt): column "txt" could not be found in any table in scope
Error processing batch

I didn't get to test this on MySQL, but the documentation examples does not have any special syntax for accessing procedure parameters.
Did I miss something or is this a dolt bug?

@cipharius cipharius changed the title Can not user stored procedure with arguments Can not use stored procedure with arguments Mar 30, 2021
@timsehn
Copy link
Contributor

timsehn commented Mar 30, 2021

Our stored procedure engine is Alpha. There is definitely syntax we don't support yet.

@Hydrocharged can give you more information.

@Hydrocharged
Copy link
Contributor

Hydrocharged commented Mar 30, 2021

This is definitely a Dolt bug! We do parameter name resolution for stored procedures in an analysis step that's independent from column name resolution. It looks like we're accidentally not looking into the VALUES of INSERT and REPLACE during the param name resolution. I'll get it fixed by the next release!

@Hydrocharged
Copy link
Contributor

@cipharius This is fixed in the above PR!

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 a pull request may close this issue.

3 participants