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

Support for materialization = view #5

Open
bdeboe opened this issue Dec 5, 2024 · 7 comments
Open

Support for materialization = view #5

bdeboe opened this issue Dec 5, 2024 · 7 comments

Comments

@bdeboe
Copy link
Contributor

bdeboe commented Dec 5, 2024

In the current dbt-iris release, when using materialization = view, it's in fact generating tables that are fully materialized. This was due to some limitations or inconveniences in the IRIS version at that time, but I believe the acute ones have been addressed and we have a customer that really needs the view option because of dataset size.

Happy to sort out any remaining wrinkles and collaborate on a solution.

@bdeboe
Copy link
Contributor Author

bdeboe commented Dec 6, 2024

Hi @daimor ,
I'm poking around with a simple approach to add support for this in my fork, but wanted to know if there are any expected unit test failures that I can safely ignore. These don't feel related to what I'm trying:

  • \tests\functional\basic\test_simple_reference.py::test_simple_reference
  • \tests\functional\shared_tests\test_concurrency.py::TestConcurrency::test_concurrency

@daimor
Copy link
Member

daimor commented Dec 10, 2024

Hi, that's great,
I'll try to find time to look at it
as far as I remember, the most issues were with renaming views, or columns in the view

@bdeboe
Copy link
Contributor Author

bdeboe commented Dec 10, 2024

I think I have the test_simple_reference case covered now, but find the test suite a little hard to navigate, so I'm not sure whether there are tests on renaming columns that are now passing, or they're still silently skipped from the parent project.

FWIW, I'm taking a rather blunt approach to renaming views, just recreating them as needed. That hasn't appeared to cause any trouble with cascaded fallout so far, but again I'm not sure whether I need to un-skip more tests from the parent project.

@daimor
Copy link
Member

daimor commented Dec 10, 2024

check out views branch
it fails mostly at rename

AS expected, IDENTIFIER (rename) found ^ alter view "test17338394554585243025_test_concurrency"."view_model__815b4eb7983b46038e85b1de28b6b559" rename>

SQL for rename defined in macros rename_relation

@bdeboe
Copy link
Contributor Author

bdeboe commented Dec 11, 2024

IRIS doesn't support an ALTER VIEW v RENAME ... command (and has no intent to add that), so the approach I took in my fork is to drop and recreate, after reading the view definition query from the dictionary tables. That seems to work (also after marking views as renamable, having found the trick in your branch), with the exception of that concurrency test, which I still don't quite understand.

@daimor
Copy link
Member

daimor commented Dec 11, 2024

I also found an issue with this test, and as far as I got, there is something to do with cascade drop, and/or clearing cache. Need to find out how to refresh cache. When the second create of table_a happens, it thinks that it's still exists. And only happens when view used instead of table

@bdeboe
Copy link
Contributor Author

bdeboe commented Dec 13, 2024

Ha, this is the pre-existing problem that ALTER TABLE t RENAME tt fails when there are views referring to t. Let me check with development where that stands and what options we'd have to work around it without requiring an IRIS server change.

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

No branches or pull requests

2 participants