Skip to content

Commit ee05a80

Browse files
committed
sql: remove unneeded isCopy field
We don't need this field, since it's only used in one conditional, and that check can be replaced by a direct check of the txn associated with the planner. Release note: None
1 parent aca6558 commit ee05a80

File tree

4 files changed

+13
-26
lines changed

4 files changed

+13
-26
lines changed

pkg/sql/conn_executor.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,7 +2789,6 @@ func (ex *connExecutor) execCopyOut(
27892789
ex.planner.maybeLogStatement(
27902790
ctx,
27912791
ex.executorType,
2792-
true, /* isCopy */
27932792
int(ex.state.mu.autoRetryCounter),
27942793
ex.extraTxnState.txnCounter,
27952794
numOutputRows,
@@ -3041,7 +3040,7 @@ func (ex *connExecutor) execCopyIn(
30413040
ex.planner.CurrentDatabase(),
30423041
)
30433042
var stats topLevelQueryStats
3044-
ex.planner.maybeLogStatement(ctx, ex.executorType, true,
3043+
ex.planner.maybeLogStatement(ctx, ex.executorType,
30453044
int(ex.state.mu.autoRetryCounter), ex.extraTxnState.txnCounter,
30463045
numInsertedRows, ex.state.mu.stmtCount,
30473046
0, /* bulkJobId */

pkg/sql/conn_executor_exec.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,6 @@ func (ex *connExecutor) execStmtInOpenState(
803803
p.maybeLogStatement(
804804
ctx,
805805
ex.executorType,
806-
false, /* isCopy */
807806
int(ex.state.mu.autoRetryCounter),
808807
ex.extraTxnState.txnCounter,
809808
0, /* rowsAffected */
@@ -1577,7 +1576,6 @@ func (ex *connExecutor) dispatchToExecutionEngine(
15771576
planner.maybeLogStatement(
15781577
ctx,
15791578
ex.executorType,
1580-
false, /* isCopy */
15811579
int(ex.state.mu.autoRetryCounter),
15821580
ex.extraTxnState.txnCounter,
15831581
ppInfo.dispatchToExecutionEngine.rowsAffected,
@@ -1605,7 +1603,6 @@ func (ex *connExecutor) dispatchToExecutionEngine(
16051603
planner.maybeLogStatement(
16061604
ctx,
16071605
ex.executorType,
1608-
false, /* isCopy */
16091606
int(ex.state.mu.autoRetryCounter),
16101607
ex.extraTxnState.txnCounter,
16111608
nonBulkJobNumRows,
@@ -2279,7 +2276,6 @@ func (ex *connExecutor) execStmtInNoTxnState(
22792276
p.maybeLogStatement(
22802277
ctx,
22812278
ex.executorType,
2282-
false, /* isCopy */
22832279
int(ex.state.mu.autoRetryCounter),
22842280
ex.extraTxnState.txnCounter,
22852281
0, /* rowsAffected */

pkg/sql/event_log.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,6 @@ type eventLogOptions struct {
166166

167167
// Additional redaction options, if necessary.
168168
rOpts redactionOptions
169-
170-
// isCopy notes whether the current event is related to COPY.
171-
isCopy bool
172169
}
173170

174171
// redactionOptions contains instructions on how to redact the SQL
@@ -281,8 +278,8 @@ func logEventInternalForSQLStatements(
281278
) error {
282279
// Inject the common fields into the payload provided by the caller.
283280
injectCommonFields := func(event logpb.EventPayload) error {
284-
if opts.isCopy {
285-
// No txn is set for COPY, so use now instead.
281+
if txn == nil {
282+
// No txn is set (e.g. for COPY or BEGIN), so use now instead.
286283
event.CommonDetails().Timestamp = timeutil.Now().UnixNano()
287284
} else {
288285
event.CommonDetails().Timestamp = txn.KV().ReadTimestamp().WallTime

pkg/sql/exec_log.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ func (s executorType) logLabel() string { return logLabels[s] }
120120
func (p *planner) maybeLogStatement(
121121
ctx context.Context,
122122
execType executorType,
123-
isCopy bool,
124123
numRetries, txnCounter, rows, stmtCount int,
125124
bulkJobId uint64,
126125
err error,
@@ -132,7 +131,7 @@ func (p *planner) maybeLogStatement(
132131
statsCollector sqlstats.StatsCollector,
133132
) {
134133
p.maybeAuditRoleBasedAuditEvent(ctx, execType)
135-
p.maybeLogStatementInternal(ctx, execType, isCopy, numRetries, txnCounter,
134+
p.maybeLogStatementInternal(ctx, execType, numRetries, txnCounter,
136135
rows, stmtCount, bulkJobId, err, queryReceived, hasAdminRoleCache,
137136
telemetryLoggingMetrics, stmtFingerprintID, queryStats, statsCollector,
138137
)
@@ -141,7 +140,6 @@ func (p *planner) maybeLogStatement(
141140
func (p *planner) maybeLogStatementInternal(
142141
ctx context.Context,
143142
execType executorType,
144-
isCopy bool,
145143
numRetries, txnCounter, rows, stmtCount int,
146144
bulkJobId uint64,
147145
err error,
@@ -235,7 +233,7 @@ func (p *planner) maybeLogStatementInternal(
235233
auditEvent := builder.BuildAuditEvent(ctx, p, eventpb.CommonSQLEventDetails{}, execDetails)
236234
entries[idx] = auditEvent
237235
}
238-
p.logEventsOnlyExternally(ctx, isCopy, entries...)
236+
p.logEventsOnlyExternally(ctx, entries...)
239237
}
240238

241239
if slowQueryLogEnabled && (
@@ -247,12 +245,12 @@ func (p *planner) maybeLogStatementInternal(
247245
switch {
248246
case execType == executorTypeExec:
249247
// Non-internal queries are always logged to the slow query log.
250-
p.logEventsOnlyExternally(ctx, isCopy, &eventpb.SlowQuery{CommonSQLExecDetails: execDetails})
248+
p.logEventsOnlyExternally(ctx, &eventpb.SlowQuery{CommonSQLExecDetails: execDetails})
251249

252250
case execType == executorTypeInternal && slowInternalQueryLogEnabled:
253251
// Internal queries that surpass the slow query log threshold should only
254252
// be logged to the slow-internal-only log if the cluster setting dictates.
255-
p.logEventsOnlyExternally(ctx, isCopy, &eventpb.SlowQueryInternal{CommonSQLExecDetails: execDetails})
253+
p.logEventsOnlyExternally(ctx, &eventpb.SlowQueryInternal{CommonSQLExecDetails: execDetails})
256254
}
257255
}
258256

@@ -267,13 +265,12 @@ func (p *planner) maybeLogStatementInternal(
267265
// see a copy of the execution on the DEV Channel.
268266
dst: LogExternally | LogToDevChannelIfVerbose,
269267
verboseTraceLevel: execType.vLevel(),
270-
isCopy: isCopy,
271268
},
272269
&eventpb.QueryExecute{CommonSQLExecDetails: execDetails})
273270
}
274271

275272
if shouldLogToAdminAuditLog {
276-
p.logEventsOnlyExternally(ctx, isCopy, &eventpb.AdminQuery{CommonSQLExecDetails: execDetails})
273+
p.logEventsOnlyExternally(ctx, &eventpb.AdminQuery{CommonSQLExecDetails: execDetails})
277274
}
278275

279276
if telemetryLoggingEnabled && !p.SessionData().TroubleshootingMode {
@@ -412,34 +409,32 @@ func (p *planner) maybeLogStatementInternal(
412409
SchemaChangerMode: p.curPlan.instrumentation.schemaChangerMode.String(),
413410
}
414411

415-
p.logOperationalEventsOnlyExternally(ctx, isCopy, &sampledQuery)
412+
p.logOperationalEventsOnlyExternally(ctx, &sampledQuery)
416413
} else {
417414
telemetryMetrics.incSkippedQueryCount()
418415
}
419416
}
420417
}
421418

422-
func (p *planner) logEventsOnlyExternally(
423-
ctx context.Context, isCopy bool, entries ...logpb.EventPayload,
424-
) {
419+
func (p *planner) logEventsOnlyExternally(ctx context.Context, entries ...logpb.EventPayload) {
425420
// The API contract for logEventsWithOptions() is that it returns
426421
// no error when system.eventlog is not written to.
427422
_ = p.logEventsWithOptions(ctx,
428423
2, /* depth: we want to use the caller location */
429-
eventLogOptions{dst: LogExternally, isCopy: isCopy},
424+
eventLogOptions{dst: LogExternally},
430425
entries...)
431426
}
432427

433428
// logOperationalEventsOnlyExternally is a helper that sets redaction
434429
// options to omit SQL Name redaction. This is used when logging to
435430
// the telemetry channel when we want additional metadata available.
436431
func (p *planner) logOperationalEventsOnlyExternally(
437-
ctx context.Context, isCopy bool, entries ...logpb.EventPayload,
432+
ctx context.Context, entries ...logpb.EventPayload,
438433
) {
439434
// The API contract for logEventsWithOptions() is that it returns
440435
// no error when system.eventlog is not written to.
441436
_ = p.logEventsWithOptions(ctx,
442437
2, /* depth: we want to use the caller location */
443-
eventLogOptions{dst: LogExternally, isCopy: isCopy, rOpts: redactionOptions{omitSQLNameRedaction: true}},
438+
eventLogOptions{dst: LogExternally, rOpts: redactionOptions{omitSQLNameRedaction: true}},
444439
entries...)
445440
}

0 commit comments

Comments
 (0)