reverted VirtualScan simplification and added join-related info to VirtualScan

[markoszah]

In the context of the inner joins work I simplified the VirtualScan info shipped between the proxy and the sdks: instead of sending the full KV VirtualScan, only the sid and pid were sent. The rational was that the full VirtualScan info is also contained in the continuation key and, at the proxy, it can be obtained from the continuation key. However, this does not work for sorting queries, because in this case, there is a separate continuation key for each shard. When a new virtual scan is created for a shard, the sdk must send a null continuation key to the proxy for the 1st batch of this scan. So, in this case, the proxy must receive the full VirtualScan separately.

[gmfeinberg]

Is this a direct reversion or a subset and what are the compatibility implications?

[markoszah]

It is not a revert of a complete checkin, if this is what you mean. It is a revert of changes done around VirtualScan. These changes were not released in any previous release, so reverting them does not introduce compatibility issues.

However, it is not only a reversion. New info has been added to the VirtualScan because of the inner joins feature. For this, compatibility is handled by bumping the query version at the sdk and the proxy and checking the version in the (de)serialization code (as usual).

THIS HAS TO BE DONE IN ALL SDKs! That's why I put all of you in the reviewers list.

[y-polonsk]

Sorry I m late for this, but one thing is that the way virtual scan written and read in NSON does not look right to me. If we now separate resume info for each table, shouldn't we write/read an array of NSON objects for each resume info? Otherwise it seems like we are writing everything together and relying on the ordering of fields to make it right. I don't think we should rely on the ordering of fields or have multiple fields with the same name in the same NSON object. It seems to me it should be read/written hierarchically like it is declared in VirtualScan class (as an array of TableResultInfo).

[markoszah]

@y-polonsk Ideally, it should be like you say, but I did this this way to avoid compatibility issues that would arise otherwise.

[y-polonsk]

Could you elaborate on what compatibility issues would arise? What if we just have separate codes for query versions >= 5 and <= 4?