You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by shivzone <gi...@git.apache.org> on 2016/02/10 22:44:20 UTC
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
GitHub user shivzone opened a pull request:
https://github.com/apache/incubator-hawq/pull/338
HAWQ-405. Fix for handling of pg_temp schema
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-hawq HAWQ-405
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/338.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #338
----
commit d431585ecd451549419081316eab7308249e4284
Author: Shivram Mani <sh...@gmail.com>
Date: 2016-02-10T21:43:04Z
HAWQ-405. Fix for handling of pg_temp schema
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-hawq/pull/338
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by GodenYao <gi...@git.apache.org>.
Github user GodenYao commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/338#discussion_r52534179
--- Diff: src/backend/catalog/namespace.c ---
@@ -1495,18 +1495,34 @@ LookupInternalNamespaceId(const char *nspname)
Oid
LookupNamespaceId(const char *nspname, Oid dboid)
{
- if(gp_upgrade_mode){
+
+ /* check for pg_temp alias */
+ if (NSPDBOID_CURRENT == dboid && strcmp(nspname, "pg_temp") == 0)
+ {
+ if (TempNamespaceValid(true))
+ return myTempNamespace;
+ /*
+ * Since this is used only for looking up existing objects, there
+ * is no point in trying to initialize the temp namespace here;
+ * and doing so might create problems for some callers.
+ * Just fall through and give the "does not exist" error.
+ */
+ }
+
+ if(gp_upgrade_mode)
--- End diff --
I am thinking we may be able to remove the entire if(gp_upgrade_mode), as there's no upgrade path from previous Pivotal commercial 1.3.x HAWQ to 2.0 (only way is to do data reloading).
So this logic probably is no longer needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by shivzone <gi...@git.apache.org>.
Github user shivzone commented on the pull request:
https://github.com/apache/incubator-hawq/pull/338#issuecomment-182641887
Added to installcheck-good
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/338#discussion_r52548075
--- Diff: src/test/regress/expected/temp.out ---
@@ -170,3 +170,19 @@ select * from whereami;
-- you can invoke a temp function explicitly, though
-- select pg_temp.whoami();
drop table public.whereami;
+create temp table temptest (row integer, count integer);
--- End diff --
I think it would be better to have the exact scenario that failed:
```
CREATE TABLE pg_temp.temptest ...
SELECT avg(pg_temp.temptest.count) ...
```
Do you think we should cover all three cases (db.table.col, table.col, col)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/338#discussion_r52531944
--- Diff: src/backend/catalog/namespace.c ---
@@ -1495,18 +1495,34 @@ LookupInternalNamespaceId(const char *nspname)
Oid
LookupNamespaceId(const char *nspname, Oid dboid)
{
- if(gp_upgrade_mode){
+
+ /* check for pg_temp alias */
+ if (NSPDBOID_CURRENT == dboid && strcmp(nspname, "pg_temp") == 0)
+ {
+ if (TempNamespaceValid(true))
+ return myTempNamespace;
+ /*
+ * Since this is used only for looking up existing objects, there
+ * is no point in trying to initialize the temp namespace here;
+ * and doing so might create problems for some callers.
+ * Just fall through and give the "does not exist" error.
+ */
+ }
+
+ if(gp_upgrade_mode)
+ {
// This code is only need at hawq2.0 upgrade, in this case the old pg_namespace doesn't
// have column nspdboid, so we report error in else clause
- if(ObjectIdGetDatum(dboid)==NSPDBOID_CURRENT){
+ if(ObjectIdGetDatum(dboid)==NSPDBOID_CURRENT)
+ {
return caql_getoid(
NULL,
cql("SELECT oid FROM pg_namespace "
" WHERE nspname = :1",
CStringGetDatum((char *) nspname)));
- }else{
- elog(ERROR, "Upgrade cann't process the namespace: %s in dbid: %i", nspname, dboid);
}
+ else
+ elog(ERROR, "Upgrade cann't process the namespace: %s in dbid: %i", nspname, dboid);
--- End diff --
typo cannot
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/338#issuecomment-182995496
+1 :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/338#discussion_r52534496
--- Diff: src/backend/catalog/namespace.c ---
@@ -1495,18 +1495,34 @@ LookupInternalNamespaceId(const char *nspname)
Oid
LookupNamespaceId(const char *nspname, Oid dboid)
{
- if(gp_upgrade_mode){
+
+ /* check for pg_temp alias */
+ if (NSPDBOID_CURRENT == dboid && strcmp(nspname, "pg_temp") == 0)
+ {
+ if (TempNamespaceValid(true))
+ return myTempNamespace;
+ /*
+ * Since this is used only for looking up existing objects, there
+ * is no point in trying to initialize the temp namespace here;
+ * and doing so might create problems for some callers.
+ * Just fall through and give the "does not exist" error.
+ */
+ }
+
+ if(gp_upgrade_mode)
--- End diff --
It should not be part of this commit though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on the pull request:
https://github.com/apache/incubator-hawq/pull/338#issuecomment-182603281
I think it would be good to add the two cases that failed to installcheck-good. I checked, and there is already a temp test suite (src/test/regress/expected/temp.out, src/test/regress/sql/temp.sql) that can be a good place for them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by hornn <gi...@git.apache.org>.
Github user hornn commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/338#discussion_r52532011
--- Diff: src/backend/catalog/namespace.c ---
@@ -1495,18 +1495,34 @@ LookupInternalNamespaceId(const char *nspname)
Oid
LookupNamespaceId(const char *nspname, Oid dboid)
{
- if(gp_upgrade_mode){
+
+ /* check for pg_temp alias */
+ if (NSPDBOID_CURRENT == dboid && strcmp(nspname, "pg_temp") == 0)
+ {
+ if (TempNamespaceValid(true))
+ return myTempNamespace;
+ /*
+ * Since this is used only for looking up existing objects, there
+ * is no point in trying to initialize the temp namespace here;
+ * and doing so might create problems for some callers.
+ * Just fall through and give the "does not exist" error.
+ */
+ }
+
+ if(gp_upgrade_mode)
--- End diff --
indentation looks off
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-hawq pull request: HAWQ-405. Fix for handling of pg_temp...
Posted by sansanichfb <gi...@git.apache.org>.
Github user sansanichfb commented on the pull request:
https://github.com/apache/incubator-hawq/pull/338#issuecomment-182618898
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---