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

jdbc: Support Connection.getClientInfo() and setClientInfo() methods #74

Closed
Totktonada opened this issue Nov 1, 2018 · 4 comments · Fixed by #191
Closed

jdbc: Support Connection.getClientInfo() and setClientInfo() methods #74

Totktonada opened this issue Nov 1, 2018 · 4 comments · Fixed by #191
Labels
jdbc
Milestone
JDBC MVP

Comments

@Totktonada
Copy link
Member

Totktonada commented Nov 1, 2018 •
edited
Loading

It is required by the JDBC standard.

Also need to support DatabaseMetaData.getClientInfoProperties() list of supported properties.
@Totktonada Totktonada added the jdbc label Nov 1, 2018
@Totktonada Totktonada added this to the JDBC MVP milestone Nov 1, 2018
@Totktonada Totktonada changed the title jdbc: Support Connection.getClientInfo and setClientInfo() methods jdbc: Support Connection.getClientInfo() and setClientInfo() methods Nov 1, 2018
nicktorwald added a commit that referenced this issue Apr 9, 2019
@nicktorwald
Raise SQLException on methods of a closed Connection
a616415 
Add missed checks on methods of the closed connection.

Closes: #72
Affects: #119, #74
nicktorwald added a commit that referenced this issue Apr 9, 2019
@nicktorwald
Raise SQLException on methods of a closed Connection
00232b7 
Add missed checks on methods of the closed connection.

Closes: #72
Affects: #119, #74
@nicktorwald
Copy link

Docs suggest next standard properties to be supported:

  • ApplicationName - The name of the application currently utilizing the connection
  • ClientUser - The name of the user that the application using the connection is performing work for. This may not be the same as the user name that was used in establishing the connection.
  • ClientHostname - The hostname of the computer the application using the connection is running on.

Does Tarantool have any replicated system table where we can keep session data?
Does it make sense to maintain this kinda table on the driver side?
Or maybe DB has built-in command like SET <prop> = <value>, I don't sure.

Anyway, to follow the spec now, we can just say - there're no supported client properties yet.
@Totktonada any comments on this?

nicktorwald added a commit that referenced this issue Apr 17, 2019
@nicktorwald
Raise SQLException on methods of a closed Connection
25c0875 
Add missed checks on methods of the closed connection.

Closes: #72
Affects: #119, #74
@nicktorwald nicktorwald mentioned this issue Apr 17, 2019
Merged
nicktorwald added a commit that referenced this issue Apr 18, 2019
@nicktorwald
Raise SQLException on methods of a closed Connection
c047043 
Add missed checks on methods of the closed connection.

Closes: #72
Affects: #119, #74
nicktorwald added a commit that referenced this issue Apr 19, 2019
@nicktorwald
Raise SQLException on methods of a closed Connection
aaa404a 
Add missed checks on methods of the closed connection.

Closes: #72
Affects: #119, #74
@Totktonada Totktonada mentioned this issue May 20, 2019
Closed
@Totktonada
Copy link
Member Author

@nicktorwald Why session local storage is needed to support properties you listed above?

  • ApplicationName seems to be entirely driver-side thing.
  • A user may call box.session.su('username') within a Lua function. But we cannot receive an information about session variables w/o call / eval. I filed Expose effective user of a current session via iproto tarantool#4226 , but let's don't consider this issue as blocked on that. We can use a user we connected with and this will be okay for most cases I think.
  • ClientHostname is IP of a client as a server see it? Or something entirely driver-side? How this is used? For logging?

I would implement those propoerties that can be implemented in more or less correct way driver-side and stop on that for now.

@nicktorwald
Copy link

nicktorwald commented May 21, 2019 •
edited
Loading

Why session local storage is needed to support properties you listed above?

The session storage is not necessary. It can be any other way to keep the properties in the database.
API says: The driver stores the value specified in a suitable location in the database. For example in a special register, session parameter, or system table column.

ApplicationName seems to be entirely driver-side thing.

See API sentence above regarding the storage places. I'm not sure about in-memory (or driver-side) case.

ClientHostname is IP of a client as a server see it? Or something entirely driver-side? How this is used? For logging?

API says: The values supplied to these methods are used for accounting, diagnostics and debugging purposes only.

I would implement those propoerties that can be implemented in more or less correct way driver-side and stop on that for now.

API: Drivers are not required to support these properties...
The current implementation is almost enough to be compatible. (getClientInfo must return null instead of an exception). So, zero supported properties are alright. I think it's enough for MVP.

FYI: postgres supports only ApplicationName and mssql supports no properties at all.

@Totktonada
Copy link
Member Author

I'm agree. It seems it is better to leave empty properties set for now and wait if there will be real need in this feature.

nicktorwald added a commit that referenced this issue May 22, 2019
@nicktorwald
Add minimal support of Statement.*ClientInfo()
8fba046 
At present, the driver doesn't support any properties and must retrieve
empty data instead of raising an error with an exception for
Statement.setClientInfo which will throw ClientInfoException for any
attempts to set a property.

Closes: #74
@nicktorwald nicktorwald mentioned this issue May 22, 2019
Merged
nicktorwald added a commit that referenced this issue Jun 4, 2019
@nicktorwald
Add minimal support of Connection.*ClientInfo()
4b3d4a9 
At present, the driver doesn't support any properties and must retrieve
empty data instead of raising an error with an exception for
Statement.setClientInfo which will throw ClientInfoException for any
attempts to set a property.

Closes: #74
nicktorwald added a commit that referenced this issue Jun 12, 2019
@nicktorwald
Add minimal support of Connection.*ClientInfo()
828c312 
At present, the driver doesn't support any properties and must retrieve
empty data instead of raising an error with an exception for
Statement.setClientInfo which will throw ClientInfoException for any
attempts to set a property.

Closes: #74
nicktorwald added a commit that referenced this issue Jun 12, 2019
@nicktorwald
Add minimal support of Connection.*ClientInfo()
6f620ce 
At present, the driver doesn't support any properties and must retrieve
empty data instead of raising an error with an exception for
Statement.setClientInfo which will throw ClientInfoException for any
attempts to set a property.

Closes: #74
nicktorwald added a commit to nicktorwald/tarantool-java that referenced this issue Jul 1, 2019
@nicktorwald
Add minimal support of Connection.*ClientInfo()
1a8e7de 
At present, the driver doesn't support any properties and must retrieve
empty data instead of raising an error with an exception for
Statement.setClientInfo which will throw ClientInfoException for any
attempts to set a property.

Closes: tarantool#74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
jdbc
Projects
None yet
Milestone
JDBC MVP
Development

Successfully merging a pull request may close this issue.

Add minimal support of Connection.*ClientInfo() tarantool/tarantool-java
2 participants
@Totktonada @nicktorwald