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

Dropping a MATERIALIZED VIEW fails in the latest versions. #20837

Closed
rstyp opened this issue Feb 26, 2024 · 4 comments · Fixed by #20892
Closed

Dropping a MATERIALIZED VIEW fails in the latest versions. #20837

rstyp opened this issue Feb 26, 2024 · 4 comments · Fixed by #20892
Assignees

Comments

@rstyp
Copy link
Contributor

rstyp commented Feb 26, 2024

If a MATERIALIZED VIEW was previously created with a storage table, and the latest version has enabled iceberg.materialized-views.hide-storage-table, attempting to DROP the MATERIALIZED VIEW will result in the following error:

java.lang.IllegalStateException: Storage location missing in definition of materialized view sample_mv
    at com.google.common.base.Preconditions.checkState(Preconditions.java:512)
    at io.trino.plugin.iceberg.catalog.hms.TrinoHiveCatalog.dropMaterializedView(TrinoHiveCatalog.java:731)
    at io.trino.plugin.iceberg.IcebergMetadata.dropMaterializedView(IcebergMetadata.java:2855)
    at io.trino.plugin.base.classloader.ClassLoaderSafeConnectorMetadata.dropMaterializedView(ClassLoaderSafeConnectorMetadata.java:1125)
    at io.trino.tracing.TracingConnectorMetadata.dropMaterializedView(TracingConnectorMetadata.java:1283)
    at io.trino.metadata.MetadataManager.dropMaterializedView(MetadataManager.java:1579)

I think this block of code should be relocated to the else block.

String storageMetadataLocation = view.getParameters().get(METADATA_LOCATION_PROP);
checkState(storageMetadataLocation != null, "Storage location missing in definition of materialized view " + viewName);
TrinoFileSystem fileSystem = fileSystemFactory.create(session);
TableMetadata metadata = TableMetadataParser.read(new ForwardingFileIo(fileSystem), storageMetadataLocation);
String storageLocation = metadata.location();
try {
fileSystem.deleteDirectory(Location.of(storageLocation));
}
catch (IOException e) {
log.warn(e, "Failed to delete storage location '%s' for materialized view '%s'", storageLocation, viewName);
}

@rstyp rstyp self-assigned this Feb 26, 2024
@hashhar
Copy link
Member

hashhar commented Feb 26, 2024

cc: @alexjo2144

@rstyp
Copy link
Contributor Author

rstyp commented Feb 27, 2024

Thanks @hashhar,@alexjo2144, will send the parch PR soon.

@alexjo2144
Copy link
Member

alexjo2144 commented Feb 29, 2024

Hey @rstyp , thanks for reporting,

I think this block of code should be relocated to the else block.

I think that's the fix as well.

@rstyp would you like to put up a fix or should I?

@alexjo2144
Copy link
Member

@rstyp I found some other problems in this code today as well. Not as bad as this one, but enough that I've put up a PR with the patch you suggested above and a fix for what I found: #20892

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

Successfully merging a pull request may close this issue.

3 participants