Improve SQL Coverage for INSERT INTO ... SELECT
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
There are a few limitations that we could overcome as needed. I'm listing all of them in a single issue for the time-being. As we decide to implement, we could create separate issues and fix them one by one.
**1. Function evaluation:**
* Citus cannot push-down volatile and stable functions. I think, is is trivial to allow to those functions if replication factor = 1.
* For volatile functions which don't refer to a column, it makes sense to evaluate the functions on the master as we do for single row INSERTs.
* For volatile functions which refer to a column or any kind of stable functions, there seems not an easy way to implement function evaluation. See #951 where @marcocitus proposes a solution such that run the subquery in one of the placements and copy the results over to the other placement.
* I'm not sure that it works with stable functions (i.e., timestampz related functions might return different results per placement, so still this doesn't look %100 correct solution)
**2. Window functions:**
* It seems fine to push down subqueries which involve window functions partitioned over the distribution key. But still, windows functions seems quite complex to understand to me. So, the future implementer should ensure that the above statement is correct for any kind of window functions.
**3. Set operations:**
* We decided not to add set operations with the current version.
* See some of the discussions around it [[1]](https://github.com/citusdata/citus/pull/859#discussion-diff-83923512R430)
* The solution would probably be to include the subquery into a new, empty subquery such that:
```SQL
INSERT INTO
raw_events_first(user_id)
(SELECT user_id FROM raw_events_first) UNION
(SELECT user_id FROM raw_events_first);
----- => replaced with the below
INSERT INTO
raw_events_first(user_id)
SELECT * FROM
(
(SELECT user_id FROM raw_events_first) UNION
(SELECT user_id FROM raw_events_first);
) t;
```
**4. GROUPING SETS:**
* We could easily allow `GROUPING SETs` which include the partition column.
* The reason for not allowing is that `GROUPING SETs` could generate `NULL` values for the partition column as [here](https://gist.github.com/aamederen/c09473c10b99526ddaac96b49b1907f2#file-insert_select_test_2-sql-L64)
* The trick I mentioned for set operations should help for this as well since we already add the target shard's range to the top-level subquery in the current implementation.
* Note that such an implementation would have the similar behaviour as the outer joins where the partition column values with `NULL` are filtered out.
* If we decide that the above behaviour is not the expected behaviour, we could add a statement such that the query fails if it returns NULL on the partition column. (I couldn't come up with such expression right now, but, it should be possible to add an expression such that query errors out if a target entry is null).
**5. CTEs:**
* It is relatively easy to implement CTEs when the SELECT query is pruned down to a single shard.
* We need to expand `IsPartitionColumnRecursive()` function to consider the CTEs.
* I'm not very sure about the case when the SELECT query is NOT pruned down to a single shard. I need to chat with @anarazel first, then I'll update the issue accordingly.
Contributor guide
Research direction
Begin by splitting the five proposed areas—functions, window functions, set operations, grouping sets, and CTEs—and read the current INSERT ... SELECT handling around IsPartitionColumnRecursive(). Review issue #951 and the discussion in pull request #859, then define tests for each supported case and the intended behavior for NULL partition values and stable functions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql
- Domain
- databases, distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 15/100