Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow property.select to override the property field name. #375

Closed
wants to merge 1 commit into from

Conversation

kapouer
Copy link
Contributor

@kapouer kapouer commented Nov 6, 2013

This is a generic way to address
#273
Important: property.select can be a function, whose
return value won't be escaped.

I'll happily add a test and modify readme.md if that PR is ok.

This is a generic way to address
dresende#273
Important: property.select can be a function, whose
return value won't be escaped.
kapouer added a commit to kapouer/node-orm2 that referenced this pull request Nov 6, 2013
This sets a default 'select' for custom types, see:
dresende#375
@dxg
Copy link
Collaborator

dxg commented Nov 14, 2013

I would call it something like mapsTo rather than column so it generalizes for non SQL stores.
As far as I can see you would like to:
1/ be able to name the model property differently than in the datastore
2/ be able to do a custom transform for a field?? I'm not sure. What is select for?

@kapouer
Copy link
Contributor Author

kapouer commented Nov 14, 2013

Yes, it kills two birds with one stone.
The second case allows customization - code that is specific to a driver.
Things like this:

orm.db.defineType('GeoJSON', {
    datastoreType: function(prop) {
        return 'geometry(Geometry, 4326)';
    },
    datastoreSelect: function(prop, helper) {
        return 'ST_AsGeoJSON(' + prop + ') AS ' + prop;
    },
    valueToProperty: function(value, prop) {
        if (value == null) return function() { return 'NULL'; };
        else if (typeof value == "string") return JSON.parse(value);
        else return value;
    },
    propertyToValue: function(value, prop) {
        if (value == null) return value;
        else return function(h) {
            return "ST_SetSRID(ST_GeomFromGeoJSON(" + h.escapeVal(JSON.stringify(value)) + "), 4326)";
        };
    }
});

@bblack
Copy link

bblack commented Nov 14, 2013

I haven't looked at any node-orm2 code so I can't comment on the content of this PR, but I am interested in getting this feature.

@dresende
Copy link
Owner

@dxg I agree with you, I would prefer something like mapsTo or mapping or something like that. Since I've merged sql-ddl-sync with this module, this also has to be passed on to it, so the correct column/property name is used when synching.

@dxg
Copy link
Collaborator

dxg commented Nov 19, 2013

We have the concept of custom datatypes - which I think covers # 2. See this test.

Point # 1 sounds like a definite one for the todo list.

@kapouer
Copy link
Contributor Author

kapouer commented Nov 19, 2013

While i understand the "datastoreSelect" function looks a bit like a trick, the fact it allows more than just
an alias is necessary in that case because it's an actual call to a conversion function
that is available in the postgis database (and quite difficult to emulate to javascript).
Of course, it binds the model to a specific database, and being able to do that while still keeping the
nice API of orm is why i chose it over other orm modules.

kapouer added a commit to kapouer/node-orm2 that referenced this pull request Dec 28, 2013
This sets a default 'select' for custom types, see:
dresende#375
@unibasil
Copy link

How can I upvote this commit? ;) Possibility of using above mentioned pair of ST_AsGeoJSON() and ST_GeomFromGeoJSON() functions over column in table is exactly what I need. It will be great to make it public.

@dxg dxg added this to the 2.1.5 milestone Mar 20, 2014
@dxg
Copy link
Collaborator

dxg commented Mar 20, 2014

I've added this to the next milestone.
Hopefully get to it soon.

@dxg
Copy link
Collaborator

dxg commented Apr 7, 2014

@kapouer , @unibasil and any other PostGIS users out there, please install the custom-properties branch by modifying your package.json:

"orm": "https://github.com/dresende/node-orm2.git#custom-properties",

(you may need to rm node_modules/orm)

There is a sample test showing the new functionality here:
custom-properties/test/integration/property-custom.js#L75.

I need to know if this works with PostGIS, or if something else needs to be done.
Consider the custom-properties branch as experimental, so other things may be broken (tests do pass though).
datastoreGet should be analogous to datastoreSelect in the example above.

@dxg
Copy link
Collaborator

dxg commented May 7, 2014

The PR has been merged. Basic PostGIS support should now be available, however I suspect more work will be needed for complete PostGIS support.

@dxg dxg closed this May 7, 2014
dxg added a commit that referenced this pull request May 8, 2014
@kapouer
Copy link
Contributor Author

kapouer commented May 9, 2014

it works quite nicely - thank you.
datastoreGet works all right.

@unibasil
Copy link

Great, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants