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

ui: removed formatting to statements on the details pages #75443

Merged
merged 2 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion pkg/ccl/serverccl/statusccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ go_test(
"//pkg/spanconfig",
"//pkg/sql/catalog/catconstants",
"//pkg/sql/catalog/descpb",
"//pkg/sql/sem/tree",
"//pkg/sql/sqlstats",
"//pkg/sql/tests",
"//pkg/testutils",
Expand Down
122 changes: 32 additions & 90 deletions pkg/ccl/serverccl/statusccl/tenant_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/spanconfig"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats"
"github.com/cockroachdb/cockroach/pkg/sql/tests"
"github.com/cockroachdb/cockroach/pkg/testutils"
Expand Down Expand Up @@ -116,35 +115,19 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
tenantStatusServer := tenant.StatusServer().(serverpb.SQLStatusServer)

type testCase struct {
stmt string
formattedStmt string
fingerprint string
formattedFingerprint string
stmt string
fingerprint string
}

testCaseTenant := []testCase{
{stmt: `CREATE DATABASE roachblog_t`},
{stmt: `SET database = roachblog_t`},
{stmt: `CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)`},
{
stmt: `CREATE DATABASE roachblog_t`,
formattedStmt: "CREATE DATABASE roachblog_t\n",
},
{
stmt: `SET database = roachblog_t`,
formattedStmt: "SET database = roachblog_t\n",
},
{
stmt: `CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)`,
formattedStmt: "CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)\n",
},
{
stmt: `INSERT INTO posts_t VALUES (1, 'foo')`,
fingerprint: `INSERT INTO posts_t VALUES (_, '_')`,
formattedStmt: "INSERT INTO posts_t VALUES (1, 'foo')\n",
formattedFingerprint: "INSERT INTO posts_t VALUES (_, '_')\n",
},
{
stmt: `SELECT * FROM posts_t`,
formattedStmt: "SELECT * FROM posts_t\n",
stmt: `INSERT INTO posts_t VALUES (1, 'foo')`,
fingerprint: `INSERT INTO posts_t VALUES (_, '_')`,
},
{stmt: `SELECT * FROM posts_t`},
}

for _, stmt := range testCaseTenant {
Expand All @@ -156,28 +139,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
require.NoError(t, err)

testCaseNonTenant := []testCase{
{stmt: `CREATE DATABASE roachblog_nt`},
{stmt: `SET database = roachblog_nt`},
{stmt: `CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)`},
{
stmt: `CREATE DATABASE roachblog_nt`,
formattedStmt: "CREATE DATABASE roachblog_nt\n",
},
{
stmt: `SET database = roachblog_nt`,
formattedStmt: "SET database = roachblog_nt\n",
},
{
stmt: `CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)`,
formattedStmt: "CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)\n",
},
{
stmt: `INSERT INTO posts_nt VALUES (1, 'foo')`,
fingerprint: `INSERT INTO posts_nt VALUES (_, '_')`,
formattedStmt: "INSERT INTO posts_nt VALUES (1, 'foo')\n",
formattedFingerprint: "INSERT INTO posts_nt VALUES (_, '_')\n",
},
{
stmt: `SELECT * FROM posts_nt`,
formattedStmt: "SELECT * FROM posts_nt\n",
stmt: `INSERT INTO posts_nt VALUES (1, 'foo')`,
fingerprint: `INSERT INTO posts_nt VALUES (_, '_')`,
},
{stmt: `SELECT * FROM posts_nt`},
}

pgURL, cleanupGoDB := sqlutils.PGUrl(
Expand Down Expand Up @@ -230,22 +199,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
err = serverutils.GetJSONProto(nonTenant, path, &nonTenantCombinedStats)
require.NoError(t, err)

checkStatements := func(t *testing.T, tc []testCase, actual *serverpb.StatementsResponse, combined bool) {
checkStatements := func(t *testing.T, tc []testCase, actual *serverpb.StatementsResponse) {
t.Helper()
var expectedStatements []string
for _, stmt := range tc {
var expectedStmt = stmt.stmt
if combined {
expectedStmt = stmt.formattedStmt
}
if stmt.fingerprint != "" {
if combined {
expectedStmt = stmt.formattedFingerprint
} else {
expectedStmt = stmt.fingerprint
}
expectedStmt = stmt.fingerprint
}

expectedStatements = append(expectedStatements, expectedStmt)
}

Expand All @@ -272,14 +233,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {

// First we verify that we have expected stats from tenants.
t.Run("tenant-stats", func(t *testing.T) {
checkStatements(t, testCaseTenant, tenantStats, false)
checkStatements(t, testCaseTenant, tenantCombinedStats, true)
checkStatements(t, testCaseTenant, tenantStats)
checkStatements(t, testCaseTenant, tenantCombinedStats)
})

// Now we verify the non tenant stats are what we expected.
t.Run("non-tenant-stats", func(t *testing.T) {
checkStatements(t, testCaseNonTenant, &nonTenantStats, false)
checkStatements(t, testCaseNonTenant, &nonTenantCombinedStats, true)
checkStatements(t, testCaseNonTenant, &nonTenantStats)
checkStatements(t, testCaseNonTenant, &nonTenantCombinedStats)
})

// Now we verify that tenant and non-tenant have no visibility into each other's stats.
Expand Down Expand Up @@ -313,29 +274,10 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) {
func testResetSQLStatsRPCForTenant(
ctx context.Context, t *testing.T, testHelper *tenantTestHelper,
) {

type testCase struct {
stmt string
formattedStmt string
}
stmts := []testCase{
{
stmt: "SELECT 1",
formattedStmt: "SELECT 1\n",
},
{
stmt: "SELECT 1, 1",
formattedStmt: "SELECT 1, 1\n",
},
{
stmt: "SELECT 1, 1, 1",
formattedStmt: "SELECT 1, 1\n",
},
}

var expectedStatements []string
for _, tc := range stmts {
expectedStatements = append(expectedStatements, tc.formattedStmt)
stmts := []string{
"SELECT 1",
"SELECT 1, 1",
"SELECT 1, 1, 1",
}

testCluster := testHelper.testCluster()
Expand Down Expand Up @@ -365,8 +307,8 @@ func testResetSQLStatsRPCForTenant(
}()

for _, stmt := range stmts {
testCluster.tenantConn(randomServer).Exec(t, stmt.stmt)
controlCluster.tenantConn(randomServer).Exec(t, stmt.stmt)
testCluster.tenantConn(randomServer).Exec(t, stmt)
controlCluster.tenantConn(randomServer).Exec(t, stmt)
}

if flushed {
Expand All @@ -383,7 +325,7 @@ func testResetSQLStatsRPCForTenant(

require.NotEqual(t, 0, len(statsPreReset.Statements),
"expected to find stats for at least one statement, but found: %d", len(statsPreReset.Statements))
ensureExpectedStmtFingerprintExistsInRPCResponse(t, expectedStatements, statsPreReset, "test")
ensureExpectedStmtFingerprintExistsInRPCResponse(t, stmts, statsPreReset, "test")

_, err = status.ResetSQLStats(ctx, &serverpb.ResetSQLStatsRequest{
ResetPersistedStats: true,
Expand Down Expand Up @@ -420,7 +362,7 @@ func testResetSQLStatsRPCForTenant(
})
require.NoError(t, err)

ensureExpectedStmtFingerprintExistsInRPCResponse(t, expectedStatements, statsFromControlCluster, "control")
ensureExpectedStmtFingerprintExistsInRPCResponse(t, stmts, statsFromControlCluster, "control")
})
}
}
Expand Down Expand Up @@ -607,10 +549,10 @@ SET DATABASE=test_db1;
SELECT * FROM test;
`)

getCreateStmtQuery := fmt.Sprintf(`
SELECT prettify_statement(indexdef, %d, %d, %d)
FROM pg_catalog.pg_indexes
WHERE tablename = 'test' AND indexname = $1`, tree.ConsoleLineWidth, tree.PrettyAlignAndDeindent, tree.UpperCase)
getCreateStmtQuery := `
SELECT indexdef
FROM pg_catalog.pg_indexes
WHERE tablename = 'test' AND indexname = $1`

// Get index usage stats and assert expected results.
resp := getTableIndexStats(testHelper, "test_db1")
Expand Down
10 changes: 2 additions & 8 deletions pkg/server/combined_statement_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,12 @@ func collectCombinedStatements(
transaction_fingerprint_id,
app_name,
aggregated_ts,
jsonb_set(
metadata,
array['query'],
to_jsonb(
prettify_statement(metadata ->> 'query', %d, %d, %d)
)
),
metadata,
statistics,
sampled_plan,
aggregation_interval
FROM crdb_internal.statement_statistics
%s`, tree.ConsoleLineWidth, tree.PrettyAlignAndDeindent, tree.UpperCase, whereClause)
%s`, whereClause)

const expectedNumDatums = 8

Expand Down
8 changes: 3 additions & 5 deletions pkg/server/index_usage_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,21 @@ func getTableIndexUsageStats(

q := makeSQLQuery()
// TODO(#72930): Implement virtual indexes on index_usages_statistics and table_indexes
q.Append(`SELECT
q.Append(`
SELECT
ti.index_id,
ti.index_name,
ti.index_type,
total_reads,
last_read,
prettify_statement(indexdef, $, $, $)
indexdef
FROM crdb_internal.index_usage_statistics AS us
JOIN crdb_internal.table_indexes AS ti ON us.index_id = ti.index_id
AND us.table_id = ti.descriptor_id
JOIN pg_catalog.pg_index AS pgidx ON indrelid = us.table_id
JOIN pg_catalog.pg_indexes AS pgidxs ON pgidxs.crdb_oid = indexrelid
AND indexname = ti.index_name
WHERE ti.descriptor_id = $::REGCLASS`,
tree.ConsoleLineWidth,
tree.PrettyAlignAndDeindent,
tree.UpperCase,
tableID,
)

Expand Down
38 changes: 11 additions & 27 deletions pkg/server/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1949,33 +1949,17 @@ func TestStatusAPICombinedStatements(t *testing.T) {
thirdServerSQL := sqlutils.MakeSQLRunner(testCluster.ServerConn(2))

statements := []struct {
stmt string
formattedStmt string
fingerprint string
formattedFingerprint string
stmt string
fingerprinted string
}{
{stmt: `CREATE DATABASE roachblog`},
{stmt: `SET database = roachblog`},
{stmt: `CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)`},
{
stmt: `CREATE DATABASE roachblog`,
formattedStmt: "CREATE DATABASE roachblog\n",
},
{
stmt: `SET database = roachblog`,
formattedStmt: "SET database = roachblog\n",
},
{
stmt: `CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)`,
formattedStmt: "CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)\n",
},
{
stmt: `INSERT INTO posts VALUES (1, 'foo')`,
formattedStmt: "INSERT INTO posts VALUES (1, 'foo')\n",
fingerprint: `INSERT INTO posts VALUES (_, '_')`,
formattedFingerprint: "INSERT INTO posts VALUES (_, '_')\n",
},
{
stmt: `SELECT * FROM posts`,
formattedStmt: "SELECT * FROM posts\n",
stmt: `INSERT INTO posts VALUES (1, 'foo')`,
fingerprinted: `INSERT INTO posts VALUES (_, '_')`,
},
{stmt: `SELECT * FROM posts`},
}

for _, stmt := range statements {
Expand Down Expand Up @@ -2032,9 +2016,9 @@ func TestStatusAPICombinedStatements(t *testing.T) {

var expectedStatements []string
for _, stmt := range statements {
var expectedStmt = stmt.formattedStmt
if stmt.fingerprint != "" {
expectedStmt = stmt.formattedFingerprint
var expectedStmt = stmt.stmt
if stmt.fingerprinted != "" {
expectedStmt = stmt.fingerprinted
}
expectedStatements = append(expectedStatements, expectedStmt)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
font-family: SFMono-Semibold;
font-size: 14px;
line-height: 1.57;
white-space: pre-line;
word-wrap: break-word;

span {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,7 @@ export const getStatementDetailsPropsFixture = (): StatementDetailsProps => ({
},
statement: {
statement:
"CREATE TABLE IF NOT EXISTS promo_codes (\n" +
" code VARCHAR NOT NULL,\n" +
" description VARCHAR NULL,\n" +
" creation_time TIMESTAMP NULL,\n" +
" expiration_time TIMESTAMP NULL,\n" +
" rules JSONB NULL,\n" +
" PRIMARY KEY (code ASC)\n" +
")",
"CREATE TABLE IF NOT EXISTS promo_codes (code VARCHAR NOT NULL, description VARCHAR NULL, creation_time TIMESTAMP NULL, expiration_time TIMESTAMP NULL, rules JSONB NULL, PRIMARY KEY (code ASC))",
stats: statementStats,
database: "defaultdb",
byNode: [
Expand Down
Loading