Skip to content

Commit 5dca572

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 5dca572

File tree

3 files changed

+185
-3
lines changed

3 files changed

+185
-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: 46 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,43 @@ 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+
final boolean isPermissionError =
104+
e.getCode() == java.net.HttpURLConnection.HTTP_FORBIDDEN
105+
&& e.getError() != null
106+
&& "accessDenied".equals(e.getError().getReason());
107+
if (!isPermissionError) {
108+
// Not a permission error, so re-throw it.
109+
throw e;
110+
}
111+
// This is a permission error. Log a warning and fall back to materializing the view.
112+
log.warn(
113+
"Failed to initiate a direct read from Materialized View '{}' due to a permission"
114+
+ " error. The service account likely lacks 'bigquery.tables.getData'"
115+
+ " permission. Falling back to re-executing the view's underlying query. This"
116+
+ " will incur additional BigQuery costs and impact performance. For optimal"
117+
+ " performance, grant the 'roles/bigquery.dataViewer' role to the principal at"
118+
+ " the dataset or table level.",
119+
BigQueryClient.fullTableName(tableDetails.getTableId()));
120+
// Execution will now fall through to the getActualTable() call below.
121+
}
122+
}
123+
95124
TableInfo actualTable = getActualTable(tableDetails, selectedFields, filter);
125+
return createReadSession(actualTable, selectedFields, filter, sessionPrepStartTime);
126+
}
96127

128+
private ReadSessionResponse createReadSession(
129+
TableInfo actualTable,
130+
ImmutableList<String> selectedFields,
131+
Optional<String> filter,
132+
Instant sessionPrepStartTime) {
97133
BigQueryReadClient bigQueryReadClient = bigQueryReadClientFactory.getBigQueryReadClient();
98134
log.info(
99135
"|creation a read session for table {}, parameters: "
@@ -125,7 +161,7 @@ public ReadSessionResponse create(
125161
config.getTraceId().ifPresent(traceId -> requestedSession.setTraceId(traceId));
126162

127163
TableReadOptions.Builder readOptions = requestedSession.getReadOptionsBuilder();
128-
if (!isInputTableAView(tableDetails)) {
164+
if (!isInputTableAView(actualTable)) {
129165
filter.ifPresent(readOptions::setRowRestriction);
130166
}
131167
readOptions.addAllSelectedFields(selectedFields);
@@ -172,7 +208,7 @@ public ReadSessionResponse create(
172208

173209
TableModifiers.Builder modifiers = TableModifiers.newBuilder();
174210

175-
if (!isInputTableAView(tableDetails)) {
211+
if (!isInputTableAView(actualTable)) {
176212
config
177213
.getSnapshotTimeMillis()
178214
.ifPresent(
@@ -203,7 +239,10 @@ public ReadSessionResponse create(
203239
if (config.isReadSessionCachingEnabled()
204240
&& getReadSessionCache().asMap().containsKey(createReadSessionRequest)) {
205241
ReadSession readSession = getReadSessionCache().asMap().get(createReadSessionRequest);
206-
log.info("Reusing read session: {}, for table: {}", readSession.getName(), table);
242+
log.info(
243+
"Reusing read session: {}, for table: {}",
244+
readSession.getName(),
245+
actualTable.getTableId());
207246
return new ReadSessionResponse(readSession, actualTable);
208247
}
209248
ReadSession readSession = bigQueryReadClient.createReadSession(createReadSessionRequest);
@@ -255,6 +294,10 @@ TableInfo getActualTable(
255294
TableInfo table, ImmutableList<String> requiredColumns, String[] filters) {
256295
TableDefinition tableDefinition = table.getDefinition();
257296
TableDefinition.Type tableType = tableDefinition.getType();
297+
298+
// By not handling MATERIALIZED_VIEW here, we allow it to be handled by the isInputTableAView()
299+
// block below, which correctly triggers the materialization fallback.
300+
258301
if (TableDefinition.Type.TABLE == tableType
259302
|| TableDefinition.Type.EXTERNAL == tableType
260303
|| TableDefinition.Type.SNAPSHOT == tableType) {
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
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 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+
String projectId = System.getenv("GOOGLE_CLOUD_PROJECT");
59+
assertThat(projectId).isNotNull();
60+
testServiceAccount =
61+
String.format(
62+
"mv-test-%s@%s.iam.gserviceaccount.com",
63+
UUID.randomUUID().toString().substring(0, 8), projectId);
64+
IntegrationTestUtils.createServiceAccount(testServiceAccount);
65+
66+
// Create a temporary dataset and grant the new SA the BQ User role
67+
// This provides permissions to run jobs (for the fallback) but not to read table data directly
68+
testDataset = "mv_read_it_" + System.nanoTime();
69+
IntegrationTestUtils.createDataset(testDataset);
70+
IntegrationTestUtils.grantServiceAccountRole(
71+
testServiceAccount, "roles/bigquery.user", testDataset);
72+
IntegrationTestUtils.grantServiceAccountRole(
73+
testServiceAccount, "roles/bigquery.metadataViewer", testDataset);
74+
75+
// Create a source table
76+
sourceTableId = TableId.of(testDataset, "source_table_" + System.nanoTime());
77+
IntegrationTestUtils.createTable(sourceTableId, "name:string, value:integer", "name,value");
78+
79+
// Create a Materialized View
80+
mvName = "test_mv_" + System.nanoTime();
81+
String createMvSql =
82+
String.format(
83+
"CREATE MATERIALIZED VIEW `%s.%s` AS SELECT name, value FROM `%s.%s`",
84+
testDataset, mvName, testDataset, sourceTableId.getTable());
85+
QueryJobConfiguration createMvJob = QueryJobConfiguration.newBuilder(createMvSql).build();
86+
bigquery.create(JobInfo.of(createMvJob)).waitFor();
87+
}
88+
89+
@After
90+
public void tearDown() {
91+
logger.removeAppender(appender);
92+
if (testDataset != null) {
93+
IntegrationTestUtils.deleteDatasetAndTables(testDataset);
94+
}
95+
if (testServiceAccount != null) {
96+
IntegrationTestUtils.deleteServiceAccount(testServiceAccount);
97+
}
98+
}
99+
100+
@Test
101+
public void testReadMaterializedView_lackingPermission_logsWarningAndFallsBack() {
102+
// This test confirms that when reading a Materialized View with a service account
103+
// that lacks `bigquery.tables.getData` permission, the connector:
104+
// 1. Logs a specific WARN message indicating the permission issue and the fallback.
105+
// 2. Successfully reads the data by falling back to materializing the view's query.
106+
107+
// Arrange: Use the dedicated service account with insufficient permissions for a direct read.
108+
String mvToRead = testDataset + "." + mvName;
109+
110+
// Act: Read the materialized view, impersonating the test service account
111+
Dataset<Row> df =
112+
spark
113+
.read()
114+
.format("bigquery")
115+
.option("viewsEnabled", "true") // Required to read any kind of view
116+
.option("impersonationServiceAccount", testServiceAccount)
117+
.load(mvToRead);
118+
119+
List<Row> result = df.collectAsList();
120+
121+
// Assert
122+
// 1. Assert that the read was successful via fallback
123+
assertThat(result).hasSize(2);
124+
List<String> names = result.stream().map(row -> row.getString(0)).collect(Collectors.toList());
125+
assertThat(names).containsExactlyElementsIn(Arrays.asList("name1", "name2"));
126+
127+
// 2. Assert that the specific warning was logged
128+
String logOutput = stringWriter.toString();
129+
assertThat(logOutput).contains("Failed to initiate a direct read from Materialized View");
130+
assertThat(logOutput)
131+
.contains("The service account likely lacks 'bigquery.tables.getData' permission");
132+
assertThat(logOutput).contains("Falling back to re-executing the view's underlying query");
133+
}
134+
}

0 commit comments

Comments
 (0)