PostgreSQL - Functions

[Papina]

Functions to be converted to PostgreSQL

[kspearrin]

formatting seems weird in this file. Looks like tabs mixed with spaces. should just be spaces.

[kspearrin]

Why is this passed in as text and then cast to TIMESTAMPTZ later?

[kspearrin]

Why is this passed as int and cast to bit later?

[Papina]

actually ive fixed that timestamp issue now, i was messing with datatypes and missed that one initially when i was passing through NULLS and having issues due to the case (ie null vs NULL)

im recasting some of the datatypes within the functions (ie int/smallint) due to the values being passed as INT and having issues.

also bit v boolean, its better/easier to have the datatype as boolean to begin with, else it has to be passed in as int, and then cast to bit in the function.

this is just a weirdness of the function when its called normally, im not sure how your C code process would call it, but with these datatypes, it has to be cast before calling the function, or the function has to do it when doing the insert/update

eg
select * from fn_some_function(1::BIT,123::SMALLINT) ;
vs
select * from fn_some_function(false,123) ;
vs

for now i've chosen boolean instead of bit, but it can be changed back if this is too much of a change to work with.

[Papina]

last update has the case fixed

not sure on int v smallint change you want
did you want the function to cast it (pass in INT value and cast with the update statement),
or the function call to cast it (pass in value::SMALLINT)?

[kspearrin]

last update has the case fixed

not sure on int v smallint change you want
did you want the function to cast it (pass in INT value and cast with the update statement),
or the function call to cast it (pass in value::SMALLINT)?

It should be like your update function where no casting is needed.

[Papina]

i've gone back and reviewed all the functions, and recast within the function to match the current table DDL. tested the removal statements for each also

[kspearrin]

i've gone back and reviewed all the functions, and recast within the function to match the current table DDL. tested the removal statements for each also

Isn't that the opposite of what I suggested? I was suggesting that you do it like your update function was originally so that casting wasn't necessary. The proper type is defined in the parameters from the beginning.

[Papina]

ok, comms problem, lets resolve it
did you want

-1/ the function code to cast it (pass in INT value and cast with the update statement),
-2/ or the function call to cast it (pass in value::SMALLINT)?

currently it's set to -1 and there is no need to cast outside the function call (just pass in the value as-is), which is simpler and my preference, and allows for extra error control code within the function later to return values if these numbers are out of range

or -2, the code that calls the function (your c code) will need to cast it beforehand else PostgreSQL will complain the function does not exist since it cant resolve int v smallint with numbers smaller than the smallint range, and PostgreSQL is very specific about datatypes, more so than SQL Server. the error control code can still be added, but would need to be on the c code side, and may not show anything useful (just could not find function(unknown) kind of error)

If it's -2, great, no problem, ill make the changes, and assume the value is passed correctly

[kspearrin]

I am suggesting option 2.

Dapper is what calls these functions from the .NET code, and I believe it will properly handle casting the type, if necessary, as it passes the value in. I have not tested this yet with Postgres, but it does with SQL Server.

Maybe I should test before we commit to one way or the other.

[kspearrin]

I changed the query to the following, which uses the same parameter types as defined in the tables. Tested it with my local server and .NET code w/ dapper and it worked fine. No casting needed.

CREATE OR REPLACE FUNCTION user_create
(
    _id                                 UUID,
    _name                               VARCHAR,
    _email                              VARCHAR,
    _email_verified                     BOOLEAN,
    _master_password                    VARCHAR,
    _master_password_hINT               VARCHAR,
    _culture                            VARCHAR,
    _security_stamp                     VARCHAR,
    _two_factor_providers               TEXT,
    _two_factor_recovery_code           VARCHAR,
    _equivalent_domains                 TEXT,
    _excluded_global_equivalent_domains TEXT,
    _account_revision_date              TIMESTAMPTZ,
    _key                                TEXT,
    _public_key                         TEXT,
    _private_key                        TEXT,
    _premium                            BOOLEAN,
    _premium_expiration_date            TIMESTAMPTZ,
    _renewal_reminder_date              TIMESTAMPTZ,
    _storage                            BIGINT,
    _max_storage_gb                     SMALLINT,
    _gateway                            SMALLINT,
    _gateway_customer_id                VARCHAR,
    _gateway_subscription_id            VARCHAR,
    _license_key                        VARCHAR,
    _kdf                                SMALLINT,
    _kdf_iterations                     INT,
    _creation_date                      TIMESTAMPTZ,
    _revision_date                      TIMESTAMPTZ
)
RETURNS VOID
LANGUAGE 'plpgsql'
AS
$$
BEGIN
    INSERT INTO "user"
    (
        id,
        name,
        email,
        email_verified,
        master_password,
        master_password_hint,
        culture,
        security_stamp,
        two_factor_providers,
        two_factor_recovery_code,
        equivalent_domains,
        excluded_global_equivalent_domains,
        account_revision_date,
        key,
        public_key,
        private_key,
        premium,
        premium_expiration_date,
        renewal_reminder_date,
        storage,
        max_storage_gb,
        gateway,
        gateway_customer_id,
        gateway_subscription_id,
        license_key,
        kdf,
        kdf_iterations,
        creation_date,
        revision_date
    )
    VALUES
    (
        _id,
        _name,
        _email,
        _email_verified,
        _master_password,
        _master_password_hint,
        _culture,
        _security_stamp,
        _two_factor_providers,
        _two_factor_recovery_code,
        _equivalent_domains,
        _excluded_global_equivalent_domains,
        _account_revision_date,
        _key,
        _public_key,
        _private_key,
        _premium,
        _premium_expiration_date,
        _renewal_reminder_date,
        _storage,
        _max_storage_gb,
        _gateway,
        _gateway_customer_id,
        _gateway_subscription_id,
        _license_key,
        _kdf,
        _kdf_iterations,
        _creation_date,
        _revision_date
    );
END
$$

kdf and gateway are both .NET enums, that inherit from a byte type. max_storage_gb is .NET type short. Both of these worked find as SMALLINT in postgres.

[Papina]

great!
ive updated the functions affected

[kspearrin]

still typo with uppercase INT here

[Papina]

fixed

[kspearrin]

well that sucks

[mnuccioarpae]

@Papina why not use a procedure then?

[Papina]

@mnuccioarpae
I don't know Dapper, but apparently it cant use procedures

[mnuccioarpae]

@Papina Me neither, but it looks strange to me since CALL conforms to the SQL standard.

[kspearrin]

do we really need these drops if we are using CREATE OR REPLACE? just seems like added headache to manage

[Papina]

from the postgres doco:

To replace the current definition of an existing function, use CREATE OR REPLACE FUNCTION. It is not possible to change the name or argument types of a function this way (if you tried, you would actually be creating a new, distinct function). Also, CREATE OR REPLACE FUNCTION will not let you change the return type of an existing function. To do that, you must drop and recreate the function. (When using OUT parameters, that means you cannot change the types of any OUT parameters except by dropping the function.)

So yes, ultimately not required once a function is confirmed as working as intended, but i'm using it currently to assist with the conversion, and not having it would be more of a headache for me. i can comment it out if you want