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

Revert array naming to pre 1202 behavior #595

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@zapov
Contributor

zapov commented Jun 29, 2016

Register Postgres internal name in oid to name map.
Alternative names are registered in name to oid map.

During unknown type lookup use current_schemas(true) which will include pg_catalog schema too.
This is required since unregistered core types will use complex lookup for arrays.

Revert array naming to pre 1202 behavior
Register Postgres internal name in oid to name map.
Alternative names are registered in name to oid map.

During unknown type lookup use current_schemas(true) which will include pg_catalog schema too.
This is required since unregistered core types will use complex lookup for arrays.
@zapov

This comment has been minimized.

Contributor

zapov commented Jun 29, 2016

This is a fix for #591 and discussed more in #333

_oidToPgName.put(oid, pgTypeName);
String internalName = rs.getString(2);
_oidToPgName.put(oid, internalName);
_pgNameToOid.put(internalName, oid);
}
_pgNameToOid.put(pgTypeName, oid);

This comment has been minimized.

@vlsi

vlsi Jul 4, 2016

Member

This sequece looks quite odd:

_pgNameToOid.put(internalName, oid);
_pgNameToOid.put(pgTypeName, oid);

This comment has been minimized.

@zapov

zapov Jul 4, 2016

Contributor

The order of that is irelevant?

that only means... map _custom to array oid
map custom[] to array oid
the important thing is map array oid to internal name (oidToPgName)

This comment has been minimized.

@vlsi

vlsi Jul 4, 2016

Member

The order of that is irelevant?

Whatever it means, code that has x = 3; x = 4; must not exists.
It should be either x = 3; or x = 4;, not both of them at the same time.

This comment has been minimized.

@zapov

zapov Jul 4, 2016

Contributor

That's hardly the same?

What it means is _custom = 12354 and custom[] = 12345
Both are correct.

This comment has been minimized.

@vlsi

vlsi Jul 4, 2016

Member

The line 380 that you've added is useless as it gets overwritten by line 382.
Useless code is a big code smell.
Am I missing something?

This comment has been minimized.

@zapov

zapov Jul 4, 2016

Contributor

Yes. You are missing something.
It's useless when looking up ordinary type.
But it's not useless when looking up array type

For array, pgTypeName = custom[]
internalName = _custom

@zapov

This comment has been minimized.

Contributor

zapov commented Jul 4, 2016

Btw. There is a sequence of operations which could result in caching of an incorrect maping.
Eg
Create type custom
Ask for array oid and cache _custom -> array oid mapping
Create type _custom
Now the mappings have changed and old array mapping is invalid

This is the reason i prefer the [] syntax which is more stable... but since I dont depend on that I can live with old behavior

@vlsi vlsi closed this in 1d8ebfc Jul 10, 2016

@vlsi vlsi added this to the 9.4.1209 milestone Jul 11, 2016

zemian pushed a commit to zemian/pgjdbc that referenced this pull request Oct 6, 2016

fix: revert array naming to pre 1202 behavior (e.g. _int4)
Register Postgres internal name in oid to name map.
Alternative names are registered in name to oid map.

During unknown type lookup use current_schemas(true) which will include pg_catalog schema too.
This is required since unregistered core types will use complex lookup for arrays.

closes pgjdbc#595
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment