mark functions as immutable

[cereum]

When using the output of squids.encode(...)in a GENERATED column, postgres was complaining that the functions needs to be immutable.

-- Example Table
CREATE TABLE public.order(
  id integer PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY,
  external_id text UNIQUE NOT NULL GENERATED ALWAYS AS (sqids.encode(ARRAY[id, 0], 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789', 5)) STORED,
  amount numeric NOT NULL,
  name text,
  created_at timestamp with time zone DEFAULT timezone('utc', now()) NOT NULL,
  updated_at timestamp with time zone DEFAULT timezone('utc', now()) NOT NULL
);

From the postgres documentation:

An IMMUTABLE function cannot modify the database and is guaranteed to return the same results given the same arguments forever. This category allows the optimizer to pre-evaluate the function when a query calls it with constant arguments. For example, a query like SELECT ... WHERE x = 2 + 2 can be simplified on sight to SELECT ... WHERE x = 4, because the function underlying the integer addition operator is marked IMMUTABLE.

TL;DR Functions marked as IMMUTABLE always return the same output for the same input and do not depend on any external state. Non-IMMUTABLE functions may depend on database state or have side effects.

Accordingly, I've marked the following functions as follows:

Function	IMMUTABLE
sqids.shuffle	Yes
sqids.checkAlphabet	Yes
sqids.toId	Yes
sqids.toNumber	Yes
sqids.encodeNumbers	Yes
sqids.decode	Yes
sqids.encode (both versions)	Yes
sqids.isBlockedId	No
sqids.defaultBlocklist	No

The tests included with the repo all pass locally on my machine with these changes.

[cereum]

Only two functions fail the sqids.blocklist_test() when marked immutable:

sqids.defaultBlocklist()
sqids.isBlockedId(id TEXT)

All other immutable functions pass the tests.

[cereum]

@kryptus36 @4kimov any thing you guys want me to change, or fundamental deficits in this PR?

[kevinmarsh]

Thanks @cereum I've just started looking into whether to adopt this library into one of my projects and noticed immutability was lacking, so appreciate this PR.

---

Technically if sqids.isBlockedId is not immutable, then I think something like sqids.encodeNumbers/sqids.encode (which calls isBlockedId) must also not be immutable. However that'd kind of rely on someone's workflow being something like pseudocode:

SELECT sqids.encode(ARRAY[1], 'abc'); -- Returns 'aa'
INSERT INTO sqids.blocklist (str) VALUES ('aa'), ('bb');
SELECT sqids.encode(ARRAY[1], 'abc');  -- Returns 'cc'

So pragmatically speaking I'd prefer to leave things like sqids.encode as IMMUTABLE as you've done, since in a normal workflow they wouldn't be changing? Alternatively maybe the library should be accurate in its IMMUTABLE descriptions and if end users need immutable versions they can create wrapper functions in their own codebase marked as IMMUTABLE

[cereum]

Thanks @cereum I've just started looking into whether to adopt this library into one of my projects and noticed immutability was lacking, so appreciate this PR.

Technically if sqids.isBlockedId is not immutable, then I think something like sqids.encodeNumbers/sqids.encode (which calls isBlockedId) must also not be immutable. However that'd kind of rely on someone's workflow being something like pseudocode:

SELECT sqids.encode(ARRAY[1], 'abc'); -- Returns 'aa'
INSERT INTO sqids.blocklist (str) VALUES ('aa'), ('bb');
SELECT sqids.encode(ARRAY[1], 'abc');  -- Returns 'cc'

So pragmatically speaking I'd prefer to leave things like sqids.encode as IMMUTABLE as you've done, since in a normal workflow they wouldn't be changing? Alternatively maybe the library should be accurate in its IMMUTABLE descriptions and if end users need immutable versions they can create wrapper functions in their own codebase marked as IMMUTABLE

These are all fair. One thing that I can think of is having a "static blocklist" version of encode in the library. Using these ids as a generated column (at least in my use case) prevents me from having to use a transaction when interacting with my db, saves a network call, and prevents conflicts.

Like you said I think that the venn diagram of people who need the blocklist to be dynamic is small.