Elasticsearch connector documentation fixes #12912
Conversation
in elasticsearch connector documentation
in elasticsearch connector documentation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@zhenxiao : Can you help take a look at this PR? Thank you ! |
There was a problem hiding this comment.
thank you @fastcatch
generally is good. I left some comments
| ``long`` ``BIGINT`` | ||
| ``string`` ``VARCHAR`` | ||
| ``text`` ``VARCHAR`` | ||
| (all others) (unsupported) |
| ``ordinalPosition`` optional integer Ordinal position of the column. | ||
| ===================== ========= ============== ============================= | ||
|
|
||
| .. _field: https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html |
There was a problem hiding this comment.
prefer keep this link, as users might not be familiar with elasticsearch type
| ``jsonPath`` optional string Json path of Elasticsearch field. | ||
| ``jsonType`` optional string Json type of Elasticsearch field. | ||
| ``name`` required string Column name of Elasticsearch field. | ||
| ``type`` required string Column type of Elasticsearch field (see second column of `data type mapping`_). |
There was a problem hiding this comment.
prefer keep the field link, and point to type mapping
| ``jsonType`` optional string Json type of Elasticsearch field. | ||
| ``name`` required string Column name of Elasticsearch field. | ||
| ``type`` required string Column type of Elasticsearch field (see second column of `data type mapping`_). | ||
| ``jsonPath`` required string Json path of Elasticsearch field (when in doubt set to the same as ``name``). |
There was a problem hiding this comment.
could we use words other than 'when in doubt'? maybe default as name?
| ``name`` required string Column name of Elasticsearch field. | ||
| ``type`` required string Column type of Elasticsearch field (see second column of `data type mapping`_). | ||
| ``jsonPath`` required string Json path of Elasticsearch field (when in doubt set to the same as ``name``). | ||
| ``jsonType`` required string Json type of Elasticsearch field (when in doubt set to the same as ``type``). |
|
This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the task, make sure you've addressed reviewer comments, and rebase on the latest master. Thank you for your contributions! |
Fixing points 2, 4 and 5 in #12880.