On 01/02/13 21:06, Stephen Allen wrote:
Replying to both Andy and Rob in this email, comments inline.

...

Initial binding looks like a mistake because an update is multiple
operations and historical at best. Remove this (via a deprecation cycle?).
Use BIND or VALUES nowadays.

Agreed.  Initial binding should be removed.  I'd like to remove it
immediately w/o a deprecation cycle because it complicates the
implementation, and would require storage layer implementers to
continue to support it.  Also would require another breaking API
change when we eventually get rid of it.

It is a user facing change, but is it used much in the wild (I would
guess probably not)?  I would guess that it is probably used a lot
more on the query side (here we should deprecate instead of remove).

I don't want to remove them from the query side - I don't see the need to. It is a long-time feature so I prefer to leave it there.

There is overlap with ParmeterizedQueryString but that isn't a complete replacement.

-----

I'm seeing
UpdateEngineFactory.create(GraphStore graphStore, Binding inputBinding, Context context)

can inputBinding be removed as well?

** DataSinks

I didn't understand what's happening with the sinks, sorry.

The sinks for insert/delete data are a place to shove the quads that
get generated by the parser.  Instead of storing them in a List<Quad>,
they instead get passed all the way to the update engine which gave us
the sink.

An update request can be more than one operations, including more than one
INSERT DATA or DELETE DATA so multiple sinks of both kinds per request?

Shouldn't the sinks be per operation?  In the writer for example:

     @Override
     public void visit(UpdateDataInsert update)
     {
         Iter.sendToSink(update.getQuads(),
                         getInsertDataSink());
     }

maybe called for several UpdateDataInsert's

Yes, we call getInsertDataSink() per INSERT_DATA operation, and expect
the storage engine to give us a new one each time.


I am going to commit the change for simplifying the UsingList.  Do you
guys have an opinion on the two options I outlined in the response to
Rob?  I like 2) from an aesthetics point of view, but I worry a lot
about having to spawn a separate thread for each update query.  It
also is a big departure from the current API.  So I will look into how
to clean up 1).

(2) is nicer at that point but the separate thread may prove to be troublesome - such designs have in the past when RDQL (!) was multithreaded. It's OK when things are running to completion but clearing up on aborts always seems to end up being messy (e.g. JVMs don't exit because of stray threads; thread build up).

So (1) for now - I thing the sink-driven design is only marginally less attractive.

Would calling it currentInsertDataSink() be better to capture the fact it isn't a getter on the UpdateVisior?

        Andy


-Stephen


Reply via email to