Skip to content

support alphanumeric sorting for dimensional columns in groupby#2393

Merged
navis merged 1 commit intoapache:masterfrom
jaehc:support-alphanumeric-dimensional-sort-in-gropu-by
Feb 16, 2016
Merged

support alphanumeric sorting for dimensional columns in groupby#2393
navis merged 1 commit intoapache:masterfrom
jaehc:support-alphanumeric-dimensional-sort-in-gropu-by

Conversation

@jaehc
Copy link
Copy Markdown
Contributor

@jaehc jaehc commented Feb 4, 2016

Currently, an orderby clause in a groupby query only supports the lexicographical ordering for dimensional columns. I think it would be useful to provide the alphanumeric ordering as well.

StringComparator is a newly introduced class to abstract two types of ordering in characters. The main routines for them have been pulled from AlphaNemericTopNMetircSpec and LexicographicalTopNMetricSpec.

The below is a piece of grouby query which sorts the page dimension in descending alphanumeric order to give an example.

  "limitSpec" : {"type" : "default"
    , "limit":5000
    , "columns": [{"dimension":"page"
                          , "direction":"descending"
                          , "dimensionOrder" : "alphanumeric"}]
  },

It would be grateful if you post any comment here. Thanks.

@fjy fjy added this to the 0.9.1 milestone Feb 5, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an alphanumeric comparator we use for topNs

can we reuse that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AlphaNumericTopNMetricSpec.java has the comparator

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind,this is a copy of that, my bad

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 5, 2016

👍

@fjy fjy added the Feature label Feb 6, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be a constant?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean like the below?

  public static final String LEXICOGRAPHIC_NAME = "lexicographic";
  public static final String ALPHANUMERIC_NAME = "alphanumeric";

  public static final LexicographicComparator LEXICOGRAPHIC = new LexicographicComparator();
  public static final AlphanumericComparator ALPHANUMERIC = new AlphanumericComparator();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@navis
Copy link
Copy Markdown
Contributor

navis commented Feb 11, 2016

👍 with minor comments.

@jaehc
Copy link
Copy Markdown
Contributor Author

jaehc commented Feb 11, 2016

squashed all commits.

navis added a commit that referenced this pull request Feb 16, 2016
…sional-sort-in-gropu-by

support alphanumeric sorting for dimensional columns in groupby (#2393)
@navis navis merged commit cd31562 into apache:master Feb 16, 2016
String json = jsonMapper.writeValueAsString(query);
Query serdeQuery = jsonMapper.readValue(json, Query.class);

System.out.println(json);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, my bad.

@vogievetsky
Copy link
Copy Markdown
Contributor

It would be great to add this to the docs should I file a separate docs issue?

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 19, 2016

@CHOIJAEHONG1, @vogievetsky makes a good point. We should have some docs for this

seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants