Expand functions while resolving dependencies

[velioglu]

Expand functions while resolving dependencies.

In order to handle function dependencies with relations, we need to expand relation graph to include functions on relation nodes as there is no direct dependency between relation and functions. We follow a similar logic to sequences.

• Add tests

[codecov]

Codecov Report

Merging #5744 (04b92aa) into master (b825232) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5744   +/-   ##
=======================================
  Coverage   92.56%   92.56%           
=======================================
  Files         223      223           
  Lines       47315    47363   +48     
=======================================
+ Hits        43797    43843   +46     
- Misses       3518     3520    +2     

[onderkalaci]

what is the meaning of deprec->objsubid == 0 && deprec->refobjsubid != 0 here?

[velioglu]

objsubid is nonzero only for table columns and zero for anything else. Since we are trying to find a dependency from the column of a table to function we've added such a check. We may omit deprec->objsubid == 0 part , but I'm more inclined to keep it to not to get any unintentional dependency.

[onderkalaci]

can you maybe add this to code? Because this is something we'd forget :)

bool IsColumnDependency = deprec->refobjsubid != 0

[onderkalaci]

When I check pg_catalog, I see that "table constraint", "default value", "rule", "index" can be possible when refobjsubid != 0 .

Do we need anything for the latter two?

select pg_identify_object_as_address(classid, objid, objsubid), pg_identify_object_as_address(refclassid, refobjid, refobjsubid) from pg_depend where refobjsubid != 0  ORDER BY objid DESC;

[onderkalaci]

Seems like we could add statistics here?

-- create statistics based on a valid function
BEGIN;

	CREATE OR REPLACE FUNCTION non_sense_func_for_index()
	RETURNS int
	LANGUAGE plpgsql IMMUTABLE AS
	$$
	BEGIN
	    return 1;
	END; 
	$$;

	CREATE TABLE t3 (
	    a   int,
	    b int
	);

	-- build ndistinct statistics on the pair of expressions (per-expression
	-- statistics are built automatically)
	CREATE STATISTICS s3 ON (a > non_sense_func_for_index()) FROM t3;

	SELECT create_distributed_table('t3', 'a');
ERROR:  function public.non_sense_func_for_index() does not exist
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
CONTEXT:  while executing command on localhost:9700

Where we seem to have a dependency:

│ pg_identify_object_as_address │ ("statistics object","{public,s3}",{})                                                                │
│ pg_identify_object_as_address │ ("table column","{public,t3,a}",{})                                                                   │

[onderkalaci]

Similarly GENERATED ALWAYS:

-- generated always
BEGIN;

	CREATE OR REPLACE FUNCTION non_sense_func_for_generated_always()
	RETURNS int
	LANGUAGE plpgsql IMMUTABLE AS
	$$
	BEGIN
	    return 1;
	END; 
	$$;

	CREATE TABLE people (
	id int,
    height_cm numeric,
    height_in numeric GENERATED ALWAYS AS (height_cm / non_sense_func_for_generated_always()) STORED
);

SELECT create_distributed_table('people', 'id');
DETAIL:  on server onderkalaci@localhost:9701 connectionId: 8
ERROR:  function public.non_sense_func_for_generated_always() does not exist
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
CONTEXT:  while executing command on localhost:9700

[velioglu]

Logic for finding dependent functions has been updated and tests above has been added.

[onderkalaci]

Can you maybe go over the CREATE TABLE syntax once more and check if we need any more objects that might depend on a function?

My notes are the following, which you might double check:

1. Partition by a function such as use something other than lower

CREATE TABLE measurement (                                                               city_id         int not null,                                                                                                  logdate         date not null,                                                                                                 peaktemp        int,                                                                                                           unitsales       int                                                                                                        ) PARTITION BY RANGE (lower(peaktemp::text));

2. function on primary key:

BEGIN;
	CREATE TABLE TABLE (
	column_1 data_type,
	column_2 data_type,
	… 
        PRIMARY KEY (column_1, column_2 * func())
);

3. Does this work fine with Citus local tables and/or foreign tables?
4. Exclude constraints (seems to work fine)
5. Indexes depending on functions (seems to work fine, would be good to have a test)
6. Can we have functions as part of foreign keys? (e.g., ALTER TABLE table ADD CONSTRAINT fkey FOREIGN KEY (measureid) REFERENCES colocated_partitioned_table(func(measureid));
7. Can we have unique constraints with function? Probably works fine, good to manaully test:

CREATE TABLE products (
    product_no integer,
    name text,
    price numeric,
    UNIQUE (func(product_no))
);

[onderkalaci]

Similarly GENERATED ALWAYS:

-- generated always
BEGIN;

	CREATE OR REPLACE FUNCTION non_sense_func_for_generated_always()
	RETURNS int
	LANGUAGE plpgsql IMMUTABLE AS
	$$
	BEGIN
	    return 1;
	END; 
	$$;

	CREATE TABLE people (
	id int,
    height_cm numeric,
    height_in numeric GENERATED ALWAYS AS (height_cm / non_sense_func_for_generated_always()) STORED
);

SELECT create_distributed_table('people', 'id');
DETAIL:  on server onderkalaci@localhost:9701 connectionId: 8
ERROR:  function public.non_sense_func_for_generated_always() does not exist
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
CONTEXT:  while executing command on localhost:9700

[onderkalaci]

I guess with MX the rule should also exists on the workers (which does not happen now). Not related to here, just noting which is already listed here: #4812

It is good that we are prepared from the function propagation front

[onderkalaci]

It seems rules show up as Normal dependency. Lets fix that and improve the test here:

SELECT objid, pg_identify_object_as_address(refclassid, refobjid, refobjsubid),deptype from pg_depend where objid = 17001;
┌───────┬───────────────────────────────────────────────────────┬─────────┐
│ objid │ pg_identify_object_as_address │ deptype │
├───────┼───────────────────────────────────────────────────────┼─────────┤
│ 17001 │ (table,"{public,table_1_for_rule}",{}) │ a │
│ 17001 │ ("table column","{public,table_2_for_rule,col_1}",{}) │ n │
│ 17001 │ (table,"{public,table_1_for_rule}",{}) │ n │
│ 17001 │ (function,"{public,func_for_rule}",{}) │ n │
└───────┴───────────────────────────────────────────────────────┴─────────┘
(4 rows)
Time: 2.108 ms

[onderkalaci]

Mostly sharing for reference.

As far as I can tell, this test already works on the master branch via the text search configuration changes.

And, note that the dependency type is Normal.

 SELECT pg_identify_object_as_address(classid, objid, objsubid),deptype from pg_depend where refobjid = 'func_for_index_predicate'::regproc;
┌───────────────────────────────────────────────────────────────┬─────────┐
│                 pg_identify_object_as_address                 │ deptype │
├───────────────────────────────────────────────────────────────┼─────────┤
│ (index,"{public,table_to_check_func_index_dep_col_2_idx}",{}) │ n       │
└───────────────────────────────────────────────────────────────┴─────────┘
(1 row)

[onderkalaci]

Maybe one more test idea, other than this, looks pretty good:

BEGIN;
 CREATE OR REPLACE FUNCTION func_in_transaction_def_with_seq(val bigint)
RETURNS int
LANGUAGE SQL AS
$$
SELECT 2 * val;
$$;
 CREATE OR REPLACE FUNCTION func_in_transaction_def_with_func(val bigint)
RETURNS int
LANGUAGE SQL AS
$$
SELECT func_in_transaction_def_with_seq(val);
$$;
 -- Function shouldn't be propagated within transaction
SELECT pg_identify_object_as_address(classid, objid, objsubid) from citus.pg_dist_object where objid = 'func_in_transaction_def_with_seq'::regproc::oid;
 CREATE SEQUENCE myseq;
 CREATE TABLE table_to_prop_func(id int, col_1 int default func_in_transaction_def_with_func(func_in_transaction_def_with_seq(nextval('myseq'))));
SELECT create_distributed_table('table_to_prop_func','id');
 -- Function should be marked as distributed after distributing the table that depends on it
SELECT pg_identify_object_as_address(classid, objid, objsubid) from citus.pg_dist_object where objid = 'func_in_transaction_def_with_seq'::regproc::oid;
COMMIT;

[onderkalaci]

for reference, we discussed that we can also follow DEPENDENCY_NORMAL here. DEPENDENCY_AUTO will follow all the things that are dependent via column. Still, there seems no harm following DEPENDENCY_NORMAL just in case there are some complex SQL that we don't know

[onderkalaci]

It seems rules show up as Normal dependency. Lets fix that and improve the test here:

SELECT objid, pg_identify_object_as_address(refclassid, refobjid, refobjsubid),deptype from pg_depend where objid = 17001;
┌───────┬───────────────────────────────────────────────────────┬─────────┐
│ objid │ pg_identify_object_as_address │ deptype │
├───────┼───────────────────────────────────────────────────────┼─────────┤
│ 17001 │ (table,"{public,table_1_for_rule}",{}) │ a │
│ 17001 │ ("table column","{public,table_2_for_rule,col_1}",{}) │ n │
│ 17001 │ (table,"{public,table_1_for_rule}",{}) │ n │
│ 17001 │ (function,"{public,func_for_rule}",{}) │ n │
└───────┴───────────────────────────────────────────────────────┴─────────┘
(4 rows)
Time: 2.108 ms