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.

Sign up for GitHub

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

Jump to bottom

cli: cluster settings omitted in debug zip #107103

Closed
mgartner opened this issue Jul 18, 2023 · 2 comments · Fixed by #107104
Closed

cli: cluster settings omitted in debug zip #107103

mgartner opened this issue Jul 18, 2023 · 2 comments · Fixed by #107104
Assignees
@maryliag
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.

Comments

@mgartner
Copy link
Collaborator

mgartner commented Jul 18, 2023 •
edited by cockroach-jira-scripts
Loading

In #104449 we added two columns to crdb_internal.cluster_settings. This breaks a query used to generate the crdb_internal.cluster_settings.txt file in debug zips:

cockroach/pkg/cli/zip_table_registry.go

Lines 220 to 229 in 145795b

"crdb_internal.cluster_settings": {
customQueryRedacted: `SELECT * FROM (
SELECT *
FROM crdb_internal.cluster_settings
WHERE type <> 's'
) UNION (
SELECT variable, '<redacted>' as value, type, public, description
FROM crdb_internal.cluster_settings g
WHERE type = 's'
)`,

As a result, any debug zips created will have empty crdb_internal.cluster_settings.txt files. Only version 23.1.5 is affected, as of today.

Jira issue: CRDB-29893
@mgartner mgartner added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 18, 2023
@mgartner
Copy link
Collaborator Author

A work around to obtain the cluster settings is to run SELECT * FROM crdb_internal.cluster_settings manually.

This was referenced Jul 18, 2023
Merged
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jul 18, 2023
@maryliag
cli: fix debug zip with new columns for cluster settings
a008565 
The addition of new columns on cluster_settings view done
on cockroachdb#104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

Fixes cockroachdb#107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings.
@maryliag maryliag self-assigned this Jul 18, 2023
@mgartner mgartner added the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Jul 18, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jul 18, 2023

Hi @mgartner, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@mgartner mgartner added the branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 label Jul 18, 2023
maryliag added a commit to maryliag/cockroach that referenced this issue Jul 18, 2023
@maryliag
cli: fix debug zip with new columns for cluster settings
68daa3b 
The addition of new columns on cluster_settings view done
on cockroachdb#104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

Fixes cockroachdb#107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings.
maryliag added a commit to maryliag/cockroach that referenced this issue Jul 18, 2023
@maryliag
cli: fix debug zip with new columns for cluster settings
9d296bf 
The addition of new columns on cluster_settings view done
on cockroachdb#104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Fixes cockroachdb#107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings. The file
`crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).
maryliag added a commit to maryliag/cockroach that referenced this issue Jul 18, 2023
@maryliag
cli: fix debug zip with new columns for cluster settings
d1d0491 
The addition of new columns on cluster_settings view done
on cockroachdb#104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Fixes cockroachdb#107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings. The file
`crdb_internal.cluster_settings.txt` in debug zips was empty
due to this bug on v23.1.5 (only version affected by this bug).
craig bot pushed a commit that referenced this issue Jul 18, 2023
@DrewKimball @rimadeodhar @maryliag
Merge #106485 #106779 #107104
06b5ba7 
106485: opt: only infer self-join equality with a key over the base table r=DrewKimball a=DrewKimball

#### opt: only infer self-join equality with a key over the base table

Self-join equality inference was added by #105214, so that the `FuncDeps`
for a self-join would include equalities between *every* pair of columns
at the same ordinal position in the base table if there was an equality
between key columns (also at the same ordinal position). However, the
key column check was performed using the FDs of the join inputs rather
than the base table's FDs. This could lead to incorrectly adding self-join
equality filters. For example, consider the following:
```
CREATE TABLE t106371 (x INT NOT NULL, y INT NOT NULL);
INSERT INTO t106371 VALUES (1, 1), (1, 2);

SELECT * FROM t106371 a JOIN t106371 b ON a.x = b.x;

SELECT * FROM (SELECT * FROM t106371 ORDER BY y DESC LIMIT 1) a
JOIN (SELECT DISTINCT ON (x) * FROM t106371) b ON a.x = b.x;
```
In the first query above, `a.x = b.x` does not consitute joining on
key columns. But in the second query, one input has one row and the
other de-duplicated by the `x` column and so `x` is a key over both
inputs. However, the query as written will select different rows for
each input - `a` will return the `(1, 2)` row, while `b` will return
the `(1, 1)` row. Inferring a `a.y = b.y` filter will incorrectly cause
the join to return no rows.

This patch fixes the problem by requiring the initial self-join
equalities to form a key over the *base* table, not just the inputs
of the join.

Fixes #106371

Release note: None

106779: acceptance: Fix DockerCSharp Test r=rimadeodhar a=rimadeodhar

This PR updates the .net framework within the csproj setup to .net 6. 
Additionally, it also fixes the CS program that we run as
a part of this test with the new npgsql DateTime
changes made as a part of npgsql 6 version update. The old datetime 
types have been deprecated and needed to be removed. 
With these fixes, the test is running successfully and can be unskipped.

Epic: none
Fixes: #86852
Release note: none

107104: cli: fix debug zip with new columns for cluster settings r=maryliag a=maryliag

The addition of new columns on cluster_settings view done on #104449 were causing debug zip fails to add the information from that table, since the query used to gather the information was doing a join and not considering the new columns.

This commit updates the query to use the explicit columns, so even if new columns are added it won't be a problem in the future. It also adds tests for all custom querys used to generate the debug zip, so this type of issue would have been caught.

The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Fixes #107103

Release note (bug fix): Debug zip now are properly showing the information from cluster_settings. The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: rimadeodhar <[email protected]>
Co-authored-by: maryliag <[email protected]>
@craig craig bot closed this as completed in be8c5c1 Jul 18, 2023
celiala pushed a commit that referenced this issue Jul 19, 2023
@maryliag @celiala
cli: fix debug zip with new columns for cluster settings
3bacdbe 
The addition of new columns on cluster_settings view done
on #104449 were causing debug zip fails to add the
information from that table, since the query used to
gather the information was doing a join and not considering
the new columns.

This commit updates the query to use the explicit columns,
so even if new columns are added it won't be a problem in the future.
It also adds tests for all custom querys used to generate the
debug zip, so this type of issue would have been caught.

The file `crdb_internal.cluster_settings.txt` in debug zips was
empty due to this bug on v23.1.5 (only version affected by this bug).

Fixes #107103

Release note (bug fix): Debug zip now are properly showing the
information from cluster_settings. The file
`crdb_internal.cluster_settings.txt` in debug zips was empty
due to this bug on v23.1.5 (only version affected by this bug).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maryliag maryliag

Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cli: fix debug zip with new columns for cluster settings maryliag/cockroach
2 participants
@maryliag @mgartner