>> Do we have the constraint that the literalizing methods must be >> defined on Sequel::Dataset itself? Is that so that adapters can add >> adapter-specific overrides? > > It's exactly so there can be adapter specific overrides.
I agree that all literalization should happen in Sequel::Dataset to allow overriding in adapters. > Your approach will not be handle the case if you want to subclass an > existing expression class and modify it to literalize differently, > which the current polymorphic approach will currently handle just > fine. I agree that the conditional breaks polymorphism and this bad. > Honestly, I think it would be cleaner to use polymorphic approach for > all objects, not just SQL::Expression instances, but that breaks with > Sequel's ideals of making the fewest changes possible to the core > classes (and never requiring the changes it makes to the core classes > by default). I agree that core classes should have minimal modifications, even at the expense of polymorphism. Currently, core classes are literalized by calling Dataset#literal(core_obj). This means no overriding for specific adapter behavior for core subclasses, though I'm not necessarily sure there is a need to support this. For subclasses of SQL::Expression, to_s(@ds) is called to literalize the object. For all other objects, .sql_literal (with no ds argument) is called. I like the idea aliasing sql_literal to to_s in SQL::Expression. This would require the addition of the required dataset argument to sql_literal. This would allow subclasses to call a subclass-specific method in Sequel::Dataset and allow for adapter specific behavior. As mentioned above, this should be the required convention. -michael --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "sequel-talk" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/sequel-talk?hl=en -~----------~----~----~----~------~----~------~--~---
