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

Postgres v11+ Native Stored Procedures #670

Closed
vitaly-t opened this issue Nov 9, 2019 · 24 comments · Fixed by #679
Closed

Postgres v11+ Native Stored Procedures #670

vitaly-t opened this issue Nov 9, 2019 · 24 comments · Fixed by #679

Comments

@vitaly-t
Copy link
Owner

vitaly-t commented Nov 9, 2019

This features is a follow-up on #665


PostgreSQL v11 introduced support for stored procedures.

Method proc needs to be re-done to account for that. Currently, it just forwards into func, which executes SELECT * FROM procName(values), and then resolves with one or no value.

This creates a confusion, moving forward, and needs to be changed, so that proc would execute the new CALL procName(values) instead, and resolve with null.

This will be a breaking change, and method proc will throw a server-side error, if executed against PostgreSQL server prior to v11. It will be the only version-sensitive method in the library.

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

I have done initial implementation in branch proc, but Travis CI gave me trouble upgrading PostgreSQL to v11 within .travis.yml. I have tried a few things, including this one, but no luck.

And so I cannot include new tests for stored procedures into automated tests.

Any help with this would be very welcome! Until then, I'm putting it on hold.

@realcarbonneau F.Y.I.

vitaly-t added a commit that referenced this issue Nov 9, 2019
vitaly-t added a commit that referenced this issue Nov 9, 2019
vitaly-t added a commit that referenced this issue Nov 9, 2019
@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

While working on this, found #671

@realcarbonneau
Copy link

@vitaly-t I have been testing your new proc, but it's not matching up the proc signature, even sending parameters as strings. I will trace and get back to you with more details.

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

@realcarbonneau I have tested it here, and it works fine. Make sure you use the latest from the proc branch, which now includes the fix for #671

@realcarbonneau
Copy link

Works for me:

CREATE TABLE test(
  id       SERIAL PRIMARY KEY NOT NULL,
  ts       TIMESTAMP WITH TIME ZONE,
  test_int INTEGER
);

CREATE OR REPLACE PROCEDURE test_proc(in_int INTEGER)
LANGUAGE 'plpgsql'
AS $BODY$
  DECLARE
    -- Nothing
  BEGIN
    -- Insert into Test
    INSERT INTO test (ts, test_int) VALUES (CLOCK_TIMESTAMP(), in_int);
  END
$BODY$;

CREATE OR REPLACE FUNCTION test_func(in_int INTEGER) RETURNS BOOLEAN
LANGUAGE 'plpgsql'
AS $BODY$
  DECLARE
    -- Nothing
  BEGIN
    -- Insert into Test
    INSERT INTO test (ts, test_int) VALUES (CLOCK_TIMESTAMP(), in_int);
    RETURN TRUE;
  END
$BODY$;
const procResult = await db.proc('test_proc', 3)
const funcResult = await db.func('test_func', 4)
2019-11-09 14:29:09.065 EST [17916] LOG:  statement: call test_proc(3)
2019-11-09 14:29:09.078 EST [17916] LOG:  statement: select * from test_func(4)

image

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

Great! Also, as per the #671 fix, if you name it something like testProc, it will be automatically double-quoted, i.e. as CALL "testProc"().

@realcarbonneau
Copy link

Doesn't seem to work for me.

CREATE OR REPLACE PROCEDURE testProc(in_int INTEGER)
LANGUAGE 'plpgsql'
AS $BODY$
  DECLARE
    -- Nothing
  BEGIN
    -- Insert into Test
    INSERT INTO test (ts, test_int) VALUES (CLOCK_TIMESTAMP(), in_int);
  END
$BODY$;
const procResult = await db.proc('test_proc', 3)
const proc2Result = await db.proc('testProc', 4)
const funcResult = await db.func('test_func', 5)
2019-11-09 15:01:52.142 EST [14864] LOG:  statement: call test_proc(3)
2019-11-09 15:01:52.162 EST [14864] LOG:  statement: call "testProc"(4)
2019-11-09 15:01:52.163 EST [14864] ERROR:  procedure testProc(integer) does not exist at character 6
2019-11-09 15:01:52.163 EST [14864] HINT:  No procedure matches the given name and argument types. You might need to add explicit type casts.
2019-11-09 15:01:52.163 EST [14864] STATEMENT:  call "testProc"(4)

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

@realcarbonneau It doesn't work for you because it thinks you declared procedure named "testProc", and not testProc. Do you know how camel case works in PostgreSQL? 😄

If you want to create a procedure in camel case, then you need to user double quotes:

CREATE OR REPLACE PROCEDURE "testProc"

@realcarbonneau
Copy link

Well it works on the db directly, so your call should work also.

But I know what you mean. If the coder wrote camelCase on the js side, you will identify this and always add quotes around it, instead of asking the coder to follow strictly the string literal "camelCase" on both sides.

If I pass the literal "char" to the db, that is different than char. I prefer that my strings are not changed. But it's your choice.

Either way, I always snake_case on the db side and camelCase in js and use the case-change if I need to translate. Therefore I am indifferent on this issue other than from a theoretical perspective that when you do invasive modifications of string literals, it should to be consistent and coherent across your framework.

image

@realcarbonneau
Copy link

You are going to curse! hahaha

FUNCTIONs are case sensitve in quotes, but not PROCEDUREs. See screenshots below. So the whole thing is useless for PROCEDUREs.

I would always pass the string literal to the db, no changing it, or else the whole this is not coherent. What are you going to do when a wise guy calls his function "testfunc" litterally? See the last screenshot. It becomes a spaghetti mess of logic, documentation an explanations.

image

image

image

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

Well it works on the db directly, so your call should work also.

Nope. It works directly because you are not using the actual camel case. You need to either use it in both places, or none at all.

FUNCTIONs are case sensitve in quotes, but not PROCEDUREs

That's not true. Something is wrong in your test. Names of functions and procedures work the same in relation to the camel case.

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 9, 2019

@realcarbonneau

Published 10.0.0 Beta.0, from the proc branch.

I don't want to merge until Travis CI is sorted for PG v11, and I don't know how long that's gonna take.

@realcarbonneau
Copy link

You are correct, there was a problem in my prior test sequences that left a lower case version of the procedure. I think that I understand that your perspective is that since JS is case-sensitive and that since pg is case-insensitive (technical everything is forced to lower-case), you will preserve the cases of technical names by quoting the strings that contain at least one upper-case when sending them to pg. There is no specific concept of camelCase in pg, as someone could choose PascalCase or whatever they want. I assume that you preserve cases into pg for all technical names across your library? Best practices for pg are to keep everything lower case, such as by using snake_case, but if your camelCase and PascalCase and other miXedCaSe friends are happy, that is what matters! :)

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 10, 2019

I think that I understand that your perspective is that since JS is case-sensitive and that since pg is case-insensitive

I'm afraid, you still do not understand it. JavaScript case-sensitivity got nothing to do with it, because we supply function/procedure name as a string always.

There is no specific concept of camelCase in pg

Actually, there is. Camel-case handling within PostgreSQL is very specific, and I suggest you read about it. Otherwise you are making incorrect assumptions.

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 10, 2019

Released in v10.0.0

@realcarbonneau
Copy link

@vitaly-t Official PostgreSQL Documentation
0 results for camelCase
0 results for camel case

I have never found any reference to camel case in the official documentation. Of course, there is some discussion of camelCase in the mailling lists and elsewhere on the internet, however, it is always in the greater context of the case-insensitivity of SQL identifiers (SQL:2016 or ISO/IEC 9075:2016) and not specific to camelCase. Technically, case-insensitivity is actually achieved by the SQL standard folding identifiers to uppercase and postgreSQL folding identifiers to lowercase.

SQL Syntax - Lexical Structure Section 4.1.1
"Quoting an identifier also makes it case-sensitive, whereas unquoted names are always folded to lower case. For example, the identifiers FOO, foo, and "foo" are considered the same by PostgreSQL, but "Foo" and "FOO" are different from these three and each other. (The folding of unquoted names to lower case in PostgreSQL is incompatible with the SQL standard, which says that unquoted names should be folded to upper case. Thus, foo should be equivalent to "FOO" not "foo" according to the standard. If you want to write portable applications you are advised to always quote a particular name or never quote it.)"

case-sensitive identifiers are an exception dealt with using quotes and the function quote_ident(string text)

ISO/IEC 9075:1992, page 100

4) An <SQL language identifier> is equivalent to an <SQL language
            identifier> in which every letter that is a lower-case letter
            is replaced by the equivalent upper-case letter or letters. This
            treatment includes determination of equivalence, representation
            in the Information and Definition Schemas, representation in the
            diagnostics area, and similar uses."

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 10, 2019

Not sure about the original, but there are tons about it on StackOverflow ;)

P.S. I'm wrapping up #673 right now :)

@realcarbonneau

@realcarbonneau
Copy link

Ok, tell me if you need help on anything else, testing or coding. :)

I am now running v12, mostly.

@vitaly-t
Copy link
Owner Author

Thank you! :) I'm all done with #673 - check it out ;)

@realcarbonneau

@vitaly-t
Copy link
Owner Author

vitaly-t commented Dec 4, 2019

Re-opening, to attempt pg v11 integration for tests yet again.

@dplewis 😉

Some magic within .travis.yml is needed.

@vitaly-t
Copy link
Owner Author

vitaly-t commented Dec 4, 2019

Now the library is finally using PostgreSQL v11 in its tests, thanks to @dplewis 😄

@vitaly-t
Copy link
Owner Author

vitaly-t commented Jan 11, 2020

Support for stored procedures has been revised in v10.3.3.

@realcarbonneau 😉

See also #692

@demian85
Copy link

Can someone please explain how to use the .proc method to call a procedure in Postgres v9.x? I updated from pg@7 to the latest version and it is failing when calling .proc. Can I use .func instead? Which should be the proper syntax?
Thanks.

@vitaly-t
Copy link
Owner Author

vitaly-t commented May 27, 2020

@demian85

Can someone please explain how to use the .proc method to call a procedure in Postgres v9.x?

You can't, the API clearly states, that function now requires PG v11 or later.

Can I use .func instead?

Yes, absolutely.

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

Successfully merging a pull request may close this issue.

3 participants