Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
96105: sql: add logic test for function dropped by DROP OWNED BY r=chengxiong-ruan a=chengxiong-ruan

With DROP FUNCTION supported in declarative schema changer, DROP OWNED BY can now drop functions.

Epic: None

Release note: None

96113: asof: fix internal error on AOST expression requiring normalization r=ajwerner a=msirek

Fixes #95604

This adds missing normalization of AS OF SYSTEM TIME expressions. Without normalization, some expressions such as range conditions may result in an internal error.

Release note (bug fix): This patch fixes internal errors which may occur on some AS OF SYSTEM TIME expressions.

96121: workflow: Cluster UI autopublishing configuration fixes r=nathanstilwell a=nathanstilwell

Using action `actions/setup-node@v3` we need to provide specific configuration for the internally generated `.npmrc`. This commit sets `registry-url` and `always-auth` to be explicit and changes the environment variable `NPM_TOKEN` to `NODE_AUTH_TOKEN` since this is what the action uses internally to set the auth token. I'm also switching back to using `npm` to publish as the setup-node action uses Yarn2 internally which can not read npm configuration and must have its own `.yarnrc.yml` generated.

A couple more improvements were made (since I was here),
- Add explicit output when a package should be published
- Check for the existence of a git tag before creating one (to prevent duplicates)
- Adds `--ignore-scripts` option to publish command to prevent duplicate builds.

Test publish - [Publish Cluster UI Pre-release #73](https://github.com/cockroachdb/cockroach/actions/runs/4027917363) (published [23.1.0-publishtest.0](https://www.npmjs.com/package/`@cockroachlabs/cluster-ui/v/23.1.0-publishtest.0)` and created [tag](https://github.com/cockroachdb/cockroach/releases/tag/%40cockroachlabs%2Fcluster-ui%4023.1.0-publishtest.0)) 

Epic: none

Release note: None

Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: Nathan Stilwell <[email protected]>
  • Loading branch information
4 people committed Jan 27, 2023
4 parents 8e82bf3 + d83102f + 0d64780 + 63aedbf commit 1236bc3
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 11 deletions.
17 changes: 12 additions & 5 deletions .github/workflows/cluster-ui-release-next.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Publish Cluster UI Pre-release
on:
workflow_dispatch:
push:
branches:
- master
Expand Down Expand Up @@ -31,10 +32,12 @@ jobs:
uses: actions/setup-node@v3
with:
node-version: 16
registry-url: 'https://registry.npmjs.org'
always-auth: true
cache: 'yarn'
cache-dependency-path: pkg/ui/workspaces/cluster-ui/yarn.lock
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

- name: Check if version is published
id: version-check
Expand All @@ -49,6 +52,8 @@ jobs:
echo "to npm. Publishing step should be skipped. 🛑"
else
echo "published=no" >> $GITHUB_OUTPUT
echo
echo "✅ Cluster UI package version $PACKAGE_VERSION should be published. ✅"
fi
- name: Build Cluster UI
Expand All @@ -62,10 +67,12 @@ jobs:
- name: Create version tag and push
if: steps.version-check.outputs.published == 'no'
run: |
TAGNAME="@cockroachlabs/cluster-ui@$(cat ./package.json | jq -r '.version')"
git tag $TAGNAME
git push origin $TAGNAME
TAGNAME="@cockroachlabs/cluster-ui@$(jq -r '.version' ./package.json)"
if ! [ $(git tag -l "$TAGNAME") ]; then
git tag $TAGNAME
git push origin $TAGNAME
fi
- name: Publish prerelease version
if: steps.version-check.outputs.published == 'no'
run: yarn publish --access public --tag next
run: npm publish --access public --tag next --ignore-scripts
17 changes: 12 additions & 5 deletions .github/workflows/cluster-ui-release.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Publish Cluster UI Release
on:
workflow_dispatch:
push:
branches:
- 'release-*'
Expand Down Expand Up @@ -31,10 +32,12 @@ jobs:
uses: actions/setup-node@v3
with:
node-version: 16
registry-url: 'https://registry.npmjs.org'
always-auth: true
cache: 'yarn'
cache-dependency-path: pkg/ui/workspaces/cluster-ui/yarn.lock
env:
NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

- name: Check if version is published
id: version-check
Expand All @@ -49,6 +52,8 @@ jobs:
echo "to npm. Publishing step should be skipped. 🛑"
else
echo "published=no" >> $GITHUB_OUTPUT
echo
echo "✅ Cluster UI package version $PACKAGE_VERSION should be published. ✅"
fi
- name: Get Branch name
Expand All @@ -67,10 +72,12 @@ jobs:
- name: Create version tag and push
if: steps.version-check.outputs.published == 'no'
run: |
TAGNAME="@cockroachlabs/cluster-ui@$(cat ./package.json | jq -r '.version')"
git tag $TAGNAME
git push origin $TAGNAME
TAGNAME="@cockroachlabs/cluster-ui@$(jq -r '.version' ./package.json)"
if ! [ $(git tag -l "$TAGNAME") ]; then
git tag $TAGNAME
git push origin $TAGNAME
fi
- name: Publish patch version
if: steps.version-check.outputs.published == 'no'
run: yarn publish --access public --tag ${{ steps.branch-name.outputs.branch }}
run: npm publish --access public --tag ${{ steps.branch-name.outputs.branch }} --ignore-scripts
5 changes: 5 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/as_of
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,8 @@ SET TRANSACTION AS OF system time '-1s'

statement ok
ROLLBACK

# Verify that an expression that requires normalization does not result in an
# internal error.
statement error pq: AS OF SYSTEM TIME: expected timestamp, decimal, or interval, got bool
SELECT * FROM t AS OF SYSTEM TIME ('' BETWEEN '' AND '')
17 changes: 17 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/drop_owned_by
Original file line number Diff line number Diff line change
Expand Up @@ -633,3 +633,20 @@ GRANT SYSTEM MODIFYCLUSTERSETTING TO testuser

statement error pq: cannot perform drop owned by if role has synthetic privileges; testuser has entries in system.privileges
DROP OWNED BY testuser

subtest drop_function

statement ok
CREATE USER u_drop_udf;

statement ok
CREATE FUNCTION f_drop_udf() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;

statement ok
ALTER FUNCTION f_drop_udf OWNER TO u_drop_udf;

statement ok
DROP OWNED BY u_drop_udf;

statement error pq: unknown function: f_drop_udf\(\): function undefined
SHOW CREATE FUNCTION f_drop_udf;
1 change: 1 addition & 0 deletions pkg/sql/sem/asof/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ go_library(
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/normalize",
"//pkg/sql/sem/tree",
"//pkg/sql/types",
"//pkg/util/duration",
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/sem/asof/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/normalize"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/duration"
Expand Down Expand Up @@ -192,7 +193,10 @@ func Eval(
return eval.AsOfSystemTime{}, newInvalidExprError()
}
}

var err error
if te, err = normalize.Expr(ctx, evalCtx, te); err != nil {
return eval.AsOfSystemTime{}, err
}
d, err := eval.Expr(ctx, evalCtx, te)
if err != nil {
return eval.AsOfSystemTime{}, err
Expand Down

0 comments on commit 1236bc3

Please sign in to comment.