CASSGO-92: Add a public method to retrieve StatementMetadata and LogField methods

[jameshartig]

StatementMetadata can be called on a session to get the bind, result, and pk information for a given query. Previously this wasn't publicly exposed but is necessary for some implementations of HostSelectionPolicy like token-aware. This might also be useful for CI tooling or runtime analysis of queries and the types of columns.

NewLogField* are methods to to return a LogField with name and a specific type.

Finally, session init was cleaned up to prevent a HostSelectionPolicy from causing a panic if it tried to make a query during init. The interface was documented that queries should not be attempted.

Patch by James Hartig for CASSGO-92

[joao-r-reis]

Allowing queries to be executed before the session is even initialized is so confusing to me, the session doesn't even have a control connection until it is initialized. I understand the motivation behind letting policies query something in the DB to initialize but I feel like if such use case does exist it should be better to add a new method to the policy called OnSessionInit or something that will be called by the session after its initialization has completed. However adding a new method to the interface is a breaking change so we'd have to add a new interface and type check if the policy implements the new interface

[joao-r-reis]

I feel like this use case is so niche that it would be better to tell those users to implement a policy with "two states" and the user is responsible for changing the state of that policy when they create a session

[jameshartig]

Its not obvious right now because it's called Init and it's passed a session but it doesn't say the session isn't ready or isn't usable. If the policy is ready there isn't a reason why the policy should have assumed the session isn't.

I'm not arguing that we change things but we should at least document the intention of init and the state of the session.

[joao-r-reis]

yeah honestly it doesn't make a lot of sense that we're passing the Session in the Init method because it's not clear exactly which methods of the Session type API can actually be used by the policy at that point. I think the best we can do at this point is to document this in the the Init method docs because any other way of addressing this probably requires a breaking change

[jameshartig]

I updated the interface to clarify Init. That said, I want to keep the changes I made since they're small and prevent the panic. In a host policy I was debugging, it starts a background goroutine to refresh the partitions and if that runs too soon it could still cause a panic without these changes.

[joao-r-reis]

👍 I'm actually curious to know what happens now with these changes, there is an executor but there is no hosts or connection pools, the ideal fix would be for the policy to get notified somehow that the session has finished initializing. Ideally the driver would do that on its own but without this I guess the user creating the policy has to write that code on the app side

In any case we can keep these changes sure

[joao-r-reis]

the description of CASSGO-92 has a few more things than were included here in this PR, maybe we should make CASSGO-92 an Epic and create a JIRA for what's included here and possibly other JIRAs to address the remaining points of CASSGO-92. Or maybe keep CASSGO-92 as is and just create sub tasks?

[jameshartig]

the description of CASSGO-92 has a few more things than were included here in this PR, maybe we should make CASSGO-92 an Epic and create a JIRA for what's included here and possibly other JIRAs to address the remaining points of CASSGO-92. Or maybe keep CASSGO-92 as is and just create sub tasks?

I wasn't necessarily intending on it being realistic to fix all of the things I brought up especially the ones with workarounds. Could I just make another issue for the Context() method on ExecutableStatement? Not sure if it's worth breaking the interface but it might be? We can decide separately but as-is it's not insurmountable.

[joao-r-reis]

I wasn't necessarily intending on it being realistic to fix all of the things I brought up especially the ones with workarounds.

That makes sense, I'd say we rename the issue to Add a public method to retrieve StatementMetadata or something like that to make it more specific as to what this PR is doing but you can keep the description that talks about the usability of the policy because that's good context that we can refer to later.

Could I just make another issue for the Context() method on ExecutableStatement? Not sure if it's worth breaking the interface but it might be? We can decide separately but as-is it's not insurmountable.

Yeah go for it. I'm also not sure about whether it's worth breaking the interface, I think I'd rather document how users can achieve that by type casting the object returned by Statement(). I added the Statement() method so that users could keep the functionality of type casting to Query or Batch which might be useful at times.

I do think we should create JIRAs for these things that require breaking changes and tag them with an appropriate label so that we can address them when we work on a 3.0. I'd say creating one to review the HostSelectionPolicy API (especially the Init method) and one to review ExecutableStatement.

[jameshartig]

@joao-r-reis updated the JIRA issue and updated this PR according to the comments.

[joao-r-reis]

👍