Skip to content

Commit 5a2801d

Browse files
committed
feat(connector): Add warning log for MV read permission fallback
When reading a Materialized View without 'bigquery.tables.getData' permission, the connector silently falls back to re-executing the view's query. This is opaque to the user and causes unexpected costs and poor performance, which are difficult to diagnose. This change introduces a try-catch block to detect a permission-denied error (HTTP 403) when attempting a direct read of a Materialized View. If this specific error is caught, a detailed WARN message is logged, explaining the cause of the fallback and instructing the user how to resolve it by granting the 'roles/bigquery.dataViewer' role. A new integration test, MaterializedViewReadIT, has been added to verify this behavior by impersonating a service account with insufficient permissions and asserting that the warning is logged. This significantly improves the supportability and user experience of the connector by making the fallback behavior transparent and empowering users to self-resolve the underlying permissions issue. Related to: https://issuetracker.google.com/296281345 Related to: dataform-co/dataform#1640 Resolves #1401
1 parent 79d320f commit 5a2801d

File tree

3 files changed

+176
-3
lines changed

3 files changed

+176
-3
lines changed

.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,8 @@ AdAdHocITSuite.scala
3838

3939
# toolchains file
4040
toolchains.xml
41+
42+
# Emacs
43+
*~
44+
\#*\#
45+
.\#*

bigquery-connector-common/src/main/java/com/google/cloud/bigquery/connector/common/ReadSessionCreator.java

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static com.google.cloud.bigquery.connector.common.BigQueryErrorCode.UNSUPPORTED;
1919
import static java.lang.String.format;
2020

21+
import com.google.cloud.bigquery.BigQueryException;
2122
import com.google.cloud.bigquery.TableDefinition;
2223
import com.google.cloud.bigquery.TableId;
2324
import com.google.cloud.bigquery.TableInfo;
@@ -92,8 +93,40 @@ public ReadSessionResponse create(
9293
TableId table, ImmutableList<String> selectedFields, Optional<String> filter) {
9394
Instant sessionPrepStartTime = Instant.now();
9495
TableInfo tableDetails = bigQueryClient.getTable(table);
96+
97+
if (tableDetails.getDefinition().getType() == TableDefinition.Type.MATERIALIZED_VIEW) {
98+
try {
99+
// Attempt to read the Materialized View directly. This will fail if the required
100+
// permissions are not present.
101+
return createReadSession(tableDetails, selectedFields, filter, sessionPrepStartTime);
102+
} catch (BigQueryException e) {
103+
if (e.getCode() == java.net.HttpURLConnection.HTTP_FORBIDDEN) {
104+
log.warn(
105+
"Failed to initiate a direct read from Materialized View '{}' due to a permission"
106+
+ " error. The service account likely lacks 'bigquery.tables.getData'"
107+
+ " permission. Falling back to re-executing the view's underlying query. This"
108+
+ " will incur additional BigQuery costs and impact performance. For optimal"
109+
+ " performance, grant the 'roles/bigquery.dataViewer' role to the principal at"
110+
+ " the dataset or table level.",
111+
tableDetails.getTableId().toString());
112+
// The catch block is intentionally empty. The code will now fall through to the
113+
// getActualTable() call below, which will materialize the view.
114+
} else {
115+
// It's another type of error, re-throw it
116+
throw e;
117+
}
118+
}
119+
}
120+
95121
TableInfo actualTable = getActualTable(tableDetails, selectedFields, filter);
122+
return createReadSession(actualTable, selectedFields, filter, sessionPrepStartTime);
123+
}
96124

125+
private ReadSessionResponse createReadSession(
126+
TableInfo actualTable,
127+
ImmutableList<String> selectedFields,
128+
Optional<String> filter,
129+
Instant sessionPrepStartTime) {
97130
BigQueryReadClient bigQueryReadClient = bigQueryReadClientFactory.getBigQueryReadClient();
98131
log.info(
99132
"|creation a read session for table {}, parameters: "
@@ -125,7 +158,7 @@ public ReadSessionResponse create(
125158
config.getTraceId().ifPresent(traceId -> requestedSession.setTraceId(traceId));
126159

127160
TableReadOptions.Builder readOptions = requestedSession.getReadOptionsBuilder();
128-
if (!isInputTableAView(tableDetails)) {
161+
if (!isInputTableAView(actualTable)) {
129162
filter.ifPresent(readOptions::setRowRestriction);
130163
}
131164
readOptions.addAllSelectedFields(selectedFields);
@@ -172,7 +205,7 @@ public ReadSessionResponse create(
172205

173206
TableModifiers.Builder modifiers = TableModifiers.newBuilder();
174207

175-
if (!isInputTableAView(tableDetails)) {
208+
if (!isInputTableAView(actualTable)) {
176209
config
177210
.getSnapshotTimeMillis()
178211
.ifPresent(
@@ -203,7 +236,7 @@ public ReadSessionResponse create(
203236
if (config.isReadSessionCachingEnabled()
204237
&& getReadSessionCache().asMap().containsKey(createReadSessionRequest)) {
205238
ReadSession readSession = getReadSessionCache().asMap().get(createReadSessionRequest);
206-
log.info("Reusing read session: {}, for table: {}", readSession.getName(), table);
239+
log.info("Reusing read session: {}, for table: {}", readSession.getName(), actualTable);
207240
return new ReadSessionResponse(readSession, actualTable);
208241
}
209242
ReadSession readSession = bigQueryReadClient.createReadSession(createReadSessionRequest);
@@ -255,6 +288,9 @@ TableInfo getActualTable(
255288
TableInfo table, ImmutableList<String> requiredColumns, String[] filters) {
256289
TableDefinition tableDefinition = table.getDefinition();
257290
TableDefinition.Type tableType = tableDefinition.getType();
291+
// This is the line that has been reverted to fix the bug identified in code review.
292+
// By removing MATERIALIZED_VIEW from this condition, we ensure that it is handled by the
293+
// isInputTableAView() block below, which correctly triggers the materialization fallback.
258294
if (TableDefinition.Type.TABLE == tableType
259295
|| TableDefinition.Type.EXTERNAL == tableType
260296
|| TableDefinition.Type.SNAPSHOT == tableType) {
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright 2025 Google LLC and Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.cloud.spark.bigquery.integration;
17+
18+
import static com.google.common.truth.Truth.assertThat;
19+
20+
import com.google.cloud.bigquery.JobInfo;
21+
import com.google.cloud.bigquery.QueryJobConfiguration;
22+
import com.google.cloud.bigquery.TableId;
23+
import com.google.cloud.spark.bigquery.ReadSessionCreator;
24+
import java.io.StringWriter;
25+
import java.util.Arrays;
26+
import java.util.List;
27+
import java.util.UUID;
28+
import java.util.stream.Collectors;
29+
import org.apache.log4j.Level;
30+
import org.apache.log4j.Logger;
31+
import org.apache.log4j.SimpleLayout;
32+
import org.apache.log4j.WriterAppender;
33+
import org.apache.spark.sql.Dataset;
34+
import org.apache.spark.sql.Row;
35+
import org.junit.After;
36+
import org.junit.Before;
37+
import org.junit.Test;
38+
39+
public class MaterializedViewReadIT extends SparkBigQueryIntegrationTestBase {
40+
41+
private static final Logger logger = Logger.getLogger(ReadSessionCreator.class);
42+
private WriterAppender appender;
43+
private StringWriter stringWriter;
44+
private String testDataset;
45+
private String mvName;
46+
private TableId sourceTableId;
47+
private String testServiceAccount;
48+
49+
@Before
50+
public void setUp() throws InterruptedException {
51+
// Set up a custom log appender to capture logs
52+
stringWriter = new StringWriter();
53+
appender = new WriterAppender(new SimpleLayout(), stringWriter);
54+
appender.setThreshold(Level.WARN);
55+
logger.addAppender(appender);
56+
57+
// Create a dedicated service account for this test
58+
testServiceAccount =
59+
String.format(
60+
"mv-test-%s@%s.iam.gserviceaccount.com",
61+
UUID.randomUUID().toString().substring(0, 8), System.getenv("GOOGLE_CLOUD_PROJECT"));
62+
IntegrationTestUtils.createServiceAccount(testServiceAccount);
63+
64+
// Create a temporary dataset and grant the new SA the BQ User role
65+
// This provides permissions to run jobs (for the fallback) but not to read table data directly
66+
testDataset = "mv_read_it_" + System.nanoTime();
67+
IntegrationTestUtils.createDataset(testDataset);
68+
IntegrationTestUtils.grantServiceAccountRole(
69+
testServiceAccount, "roles/bigquery.user", testDataset);
70+
IntegrationTestUtils.grantServiceAccountRole(
71+
testServiceAccount, "roles/bigquery.metadataViewer", testDataset);
72+
73+
// Create a source table
74+
sourceTableId = TableId.of(testDataset, "source_table_" + System.nanoTime());
75+
IntegrationTestUtils.createTable(sourceTableId, "name:string, value:integer", "name,value");
76+
77+
// Create a Materialized View
78+
mvName = "test_mv_" + System.nanoTime();
79+
String createMvSql =
80+
String.format(
81+
"CREATE MATERIALIZED VIEW `%s.%s` AS SELECT name, value FROM `%s.%s`",
82+
testDataset, mvName, testDataset, sourceTableId.getTable());
83+
QueryJobConfiguration createMvJob = QueryJobConfiguration.newBuilder(createMvSql).build();
84+
bigquery.create(JobInfo.of(createMvJob));
85+
}
86+
87+
@After
88+
public void tearDown() {
89+
logger.removeAppender(appender);
90+
if (testDataset != null) {
91+
IntegrationTestUtils.deleteDatasetAndTables(testDataset);
92+
}
93+
if (testServiceAccount != null) {
94+
IntegrationTestUtils.deleteServiceAccount(testServiceAccount);
95+
}
96+
}
97+
98+
@Test
99+
public void testReadMaterializedView_lackingPermission_logsWarningAndFallsBack() {
100+
// This test confirms that when reading a Materialized View with a service account
101+
// that lacks `bigquery.tables.getData` permission, the connector:
102+
// 1. Logs a specific WARN message indicating the permission issue and the fallback.
103+
// 2. Successfully reads the data by falling back to materializing the view's query.
104+
105+
// Arrange: Use the dedicated service account with insufficient permissions for a direct read.
106+
String mvToRead = testDataset + "." + mvName;
107+
108+
// Act: Read the materialized view, impersonating the test service account
109+
Dataset<Row> df =
110+
spark
111+
.read()
112+
.format("bigquery")
113+
.option("viewsEnabled", "true") // Required to read any kind of view
114+
.option("impersonationServiceAccount", testServiceAccount)
115+
.load(mvToRead);
116+
117+
List<Row> result = df.collectAsList();
118+
119+
// Assert
120+
// 1. Assert that the read was successful via fallback
121+
assertThat(result).hasSize(2);
122+
List<String> names = result.stream().map(row -> row.getString(0)).collect(Collectors.toList());
123+
assertThat(names).containsExactlyElementsIn(Arrays.asList("name1", "name2"));
124+
125+
// 2. Assert that the specific warning was logged
126+
String logOutput = stringWriter.toString();
127+
assertThat(logOutput).contains("Failed to initiate a direct read from Materialized View");
128+
assertThat(logOutput)
129+
.contains("The service account likely lacks 'bigquery.tables.getData' permission");
130+
assertThat(logOutput).contains("Falling back to re-executing the view's underlying query");
131+
}
132+
}

0 commit comments

Comments
 (0)