Re: [Monotone-devel] [PATCH] New typesafe VA_ARGS replacement for database with operator % style

[Nathaniel Smith]

Right, my apologies yet again for taking so long to get back to this,
just trying to catch up on things again.  This branch has been more
trouble than it really should have been, mostly my fault... but I
think we're almost there!

On Wed, Jan 25, 2006 at 09:14:54AM +0100, Christof Petig wrote:
> Actually the code got much simpler by a decent blob infrastructure being
> present in monotone. So IIRC only some minor points remain:
> 
> - The dumping routine's callback is still difficult to understand. A
> (future) rewrite can help here.
> - There's no unzip (unpack without unbase64) database function

These both seem quite trivial to fix?  And no-one will remember to do
them later...

> - I did not touch t_migrate_schema.at and I still think that's ok
> (refering to Tims comment)

I think the rule is if you add a migration function, then you touch
t_migrate_schema.at (following the instructions at the top of that
file).  I don't know why there would ever be an exception to this, but
I missed your discussion earlier... why would it be okay in this case?

> - Using blob access to results might pose a threat to platforms where
> utf16 is used for text within sqlite. No I do not want to design a
> scheme where the caller of fetch specifies whether there's text or blob
> within a given result column. Perhaps asking sqlite for the result type
> would be a good idea (like in the dump function). [I only suspect Win32
> to maybe use utf16.]

I think the answer here is "monotone doesn't use utf16, who cares what
the platform uses".

> - monotone still does not use affinities (sql column types)

Obviously this isn't necessary; but is there some benefit to it?

> Done:
> - I removed unpack and unbase64 since they are no longer needed.
> - I removed the needless WHERE conditions on schema migration
> - Vinzenz removed the need for vector and the string inheritance

Wonderful!  I notice that sqlite3_{column,value}_text_s look unused
now.

Also, the change to t_sql_unpack.at is strange... XFAIL is for tests
that are currently broken, but should be made to work in the future.
There's no point in keeping around tests for functionality that has
been flat-out removed.  Better to just delete the test entirely, or
better, turn it into a test for unzip/inflate/whatever (with
appropriate name).

So I think what's left is just:
  -- these few tweaks
  -- getting some decision on the deflate question (Matt?  Want to
     just pick one?  It is not important enough to have a long
     discussion over, I think :-).)
  -- figuring out whether to land before or after 0.26 (which I think
     depends on how much time it takes to get rid of the other
     blockers (mostly the root suturing stuff and roster_merge tests,
     though seriously, we need to get a fix for this dumb IPv6 thing
     in at some point...)

Thanks for keeping on this,
-- Nathaniel

-- 
"...these, like all words, have single, decontextualized meanings: everyone
knows what each of these words means, everyone knows what constitutes an
instance of each of their referents.  Language is fixed.  Meaning is
certain.  Santa Claus comes down the chimney at midnight on December 24."
  -- The Language War, Robin Lakoff